Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[master] fix #1596 Major performance issue in case of many new entities #1843

Merged
merged 3 commits into from
Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public void prepareTest() {
case CASE_NEW: {
selectionObject = newEmployee;
if (shouldCheckCacheByExactPrimaryKey()) {
expectedGetIdCallCount = 1;
expectedGetIdCallCount = 0;
} else {
expectedGetIdCallCount = n + 1;
}
Expand Down Expand Up @@ -192,7 +192,7 @@ public void prepareTest() {
// S.M. This went from 5 calls to 4, which is good.
// When checking the one new object + registration +
// building clone + building backup clone.
expectedGetIdCallCount = 3;
expectedGetIdCallCount = 2;
} else {
expectedGetIdCallCount = n + 4;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,11 @@ public class UnitOfWorkImpl extends AbstractSession implements org.eclipse.persi
protected Map<Object, Object> cloneMapping;
protected Map<Object, Object> newObjectsCloneToOriginal;
protected Map<Object, Object> newObjectsOriginalToClone;

/**
* Map of primary key to list of new objects. Used to speedup in-memory querying.
*/
protected Map<Object, List<Object>> primaryKeyToNewObjects;
/**
* Stores a map from the clone to the original merged object, as a different instance is used as the original for merges.
*/
Expand Down Expand Up @@ -544,6 +549,8 @@ public Object assignSequenceNumber(Object object, ClassDescriptor descriptor) th
ObjectBuilder builder = descriptor.getObjectBuilder();
try {
value = builder.assignSequenceNumber(object, this);
getPrimaryKeyToNewObjects().putIfAbsent(value, new ArrayList<>());
getPrimaryKeyToNewObjects().get(value).add(object);
} catch (RuntimeException exception) {
handleException(exception);
} finally {
Expand Down Expand Up @@ -580,6 +587,7 @@ public void assignSequenceNumbers() throws DatabaseException {
}
if (hasNewObjects()) {
assignSequenceNumbers(getNewObjectsCloneToOriginal());
setupPrimaryKeyToNewObjects();
}
}

Expand Down Expand Up @@ -2394,6 +2402,18 @@ public Map getNewObjectsCloneToOriginal() {
return newObjectsCloneToOriginal;
}

/**
* INTERNAL:
* The primaryKeyToNewObjects stores a list of objects for every primary key.
* It is used to speed up in-memory-querying.
*/
protected Map<Object, List<Object>> getPrimaryKeyToNewObjects() {
if (primaryKeyToNewObjects == null) {
primaryKeyToNewObjects = new HashMap<>();
}
return primaryKeyToNewObjects;
}

/**
* INTERNAL:
* The stores a map from new object clones to the original object used from merge.
Expand Down Expand Up @@ -2544,15 +2564,12 @@ public Object getObjectFromNewObjects(Class<?> theClass, Object selectionKey) {
boolean readSubclassesOrNoInheritance = (!descriptor.hasInheritance() || descriptor.getInheritancePolicy().shouldReadSubclasses());

ObjectBuilder objectBuilder = descriptor.getObjectBuilder();
for (Iterator newObjectsEnum = getNewObjectsCloneToOriginal().keySet().iterator();
newObjectsEnum.hasNext();) {
for (Iterator newObjectsEnum = getPrimaryKeyToNewObjects().getOrDefault(selectionKey, List.of()).iterator();
rfelcman marked this conversation as resolved.
Show resolved Hide resolved
newObjectsEnum.hasNext(); ) {
Object object = newObjectsEnum.next();
// bug 327900
if ((object.getClass() == theClass) || (readSubclassesOrNoInheritance && (theClass.isInstance(object)))) {
Object primaryKey = objectBuilder.extractPrimaryKeyFromObject(object, this, true);
if ((primaryKey != null) && primaryKey.equals(selectionKey)) {
return object;
}
}
}
return null;
Expand Down Expand Up @@ -4480,6 +4497,7 @@ protected void registerNewObjectClone(Object clone, Object original, ClassDescri
registerNewObjectInIdentityMap(clone, original, descriptor);

getNewObjectsCloneToOriginal().put(clone, original);
addNewObjectToPrimaryKeyToNewObjects(clone,descriptor);
if (original != null) {
getNewObjectsOriginalToClone().put(original, clone);
}
Expand Down Expand Up @@ -5000,6 +5018,52 @@ public void setMergeManager(MergeManager mergeManager) {
*/
protected void setNewObjectsCloneToOriginal(Map newObjects) {
this.newObjectsCloneToOriginal = newObjects;
setupPrimaryKeyToNewObjects();
}

/**
* INTERNAL:
* Create the map of primary key to new objects used to speed up in-memory querying.
*/
protected void setupPrimaryKeyToNewObjects() {
primaryKeyToNewObjects = null;
if (hasNewObjects()) {
primaryKeyToNewObjects = new HashMap<>();
getNewObjectsCloneToOriginal().forEach((object, o2) -> {
addNewObjectToPrimaryKeyToNewObjects(object);
});
}
}

/**
* INTERNAL:
* Extracts the primary key from a new object and puts it in primaryKeyToNewObjects.
*/
protected void addNewObjectToPrimaryKeyToNewObjects(Object newObject){
ClassDescriptor descriptor = getDescriptor(newObject.getClass());
addNewObjectToPrimaryKeyToNewObjects(newObject,descriptor);
}

/**
* INTERNAL:
* Extracts the primary key from a new object and puts it in primaryKeyToNewObjects.
* Allows passing of the ClassDescriptor.
*/
protected void addNewObjectToPrimaryKeyToNewObjects(Object newObject, ClassDescriptor descriptor){
ObjectBuilder objectBuilder = descriptor.getObjectBuilder();
Object pk = objectBuilder.extractPrimaryKeyFromObject(newObject, this, true);
if(pk!=null) {
getPrimaryKeyToNewObjects().putIfAbsent(pk, new ArrayList<>());
getPrimaryKeyToNewObjects().get(pk).add(newObject);
}
}

/**
* INTERNAL:
* Removes an object from primaryKeyToNewObjects.
*/
protected void removeObjectFromPrimaryKeyToNewObjects(Object object, Object primaryKey){
getPrimaryKeyToNewObjects().getOrDefault(primaryKey, new ArrayList<>(0)).remove(object);
rfelcman marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down Expand Up @@ -5459,6 +5523,7 @@ public void resumeUnitOfWork() {
}
this.newObjectsCloneToOriginal = null;
this.newObjectsOriginalToClone = null;
this.primaryKeyToNewObjects = null;
}
this.unregisteredExistingObjects = null;
this.unregisteredNewObjects = null;
Expand Down Expand Up @@ -5579,6 +5644,7 @@ public void iterate(Object object) {
// PERF: Avoid initialization of new objects if none.
if (hasNewObjects()) {
Object original = getNewObjectsCloneToOriginal().remove(object);
removeObjectFromPrimaryKeyToNewObjects(object, primaryKey);
rfelcman marked this conversation as resolved.
Show resolved Hide resolved
if (original != null) {
getNewObjectsOriginalToClone().remove(original);
}
Expand Down Expand Up @@ -5942,6 +6008,7 @@ public void clear(boolean shouldClearCache) {
this.cloneMapping = null;
this.newObjectsCloneToOriginal = null;
this.newObjectsOriginalToClone = null;
this.primaryKeyToNewObjects = null;
this.deletedObjects = null;
this.allClones = null;
this.objectsDeletedDuringCommit = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,7 @@ protected void prepareForMergeIntoRemoteUnitOfWork() {

this.newObjectsOriginalToClone = originalToClone;
this.newObjectsCloneToOriginal = cloneToOriginal;
setupPrimaryKeyToNewObjects();
}

/**
Expand Down