Skip to content

Commit

Permalink
Optimize REPLACE for index-only items
Browse files Browse the repository at this point in the history
1) We re-added cascading on SAVE/UPDATE - it seems that it can be
   safely used
2) When replacing, we simply add values to the collection instead of
   persisting them manually. It looks like the persist, although
   produces no SQL command, is particularly slow because of Hibernate
   processing.
3) We do not apply item-only modifications to main prism object
   (there's no point in doing so).
  • Loading branch information
mederly committed Jul 30, 2019
1 parent 608205a commit 48ec6c7
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 90 deletions.

Large diffs are not rendered by default.

Expand Up @@ -74,17 +74,6 @@

/**
* @author lazyman
*
* TO BE RESEARCHED FURTHER:
*
* Note on cascading into extension values sets: At one time it looked like that if we wanted to avoid fetching all values when doing DELETE,
* we had to restrict cascading of SAVE/UPDATE operation to them. Otherwise it was not sufficient to remove respective extension
* values from DB, because it was necessary to remove them also from the respective collection. And this incured SELECT operation
* on all collection members.
*
* (Now it seems that this works even when cascade = ALL. That's why it needs to be researched a bit more.)
*
* For all related places, see [SAVE-CASCADE] mark.
*/
@NamedQueries({
@NamedQuery(name = "get.focusPhoto", query = "select p.photo from RFocusPhoto p where p.ownerOid = :oid"),
Expand Down Expand Up @@ -389,43 +378,48 @@ public XMLGregorianCalendar getModifyTimestamp() {

@NotQueryable
@OneToMany(fetch = FetchType.LAZY, mappedBy = "owner", orphanRemoval = true)
@Cascade({ PERSIST, MERGE, REMOVE, REFRESH, DELETE, REPLICATE, LOCK, DETACH }) // not SAVE_UPDATE - see [SAVE-CASCADE]
//@Cascade({ PERSIST, MERGE, REMOVE, REFRESH, DELETE, REPLICATE, LOCK, DETACH }) // not SAVE_UPDATE
@Cascade({ ALL} )
public Collection<ROExtLong> getLongs() {
return longs;
}

@NotQueryable
@OneToMany(fetch = FetchType.LAZY, mappedBy = "owner", orphanRemoval = true)
@Cascade({ PERSIST, MERGE, REMOVE, REFRESH, DELETE, REPLICATE, LOCK, DETACH }) // not SAVE_UPDATE - see [SAVE-CASCADE]
//@Cascade({ PERSIST, MERGE, REMOVE, REFRESH, DELETE, REPLICATE, LOCK, DETACH }) // not SAVE_UPDATE
@Cascade({ ALL} )
public Collection<ROExtBoolean> getBooleans() {
return booleans;
}

@NotQueryable
@OneToMany(fetch = FetchType.LAZY, mappedBy = "owner", orphanRemoval = true)
@Cascade({ PERSIST, MERGE, REMOVE, REFRESH, DELETE, REPLICATE, LOCK, DETACH }) // not SAVE_UPDATE - see [SAVE-CASCADE]
//@Cascade({ ALL} )
//@Cascade({ PERSIST, MERGE, REMOVE, REFRESH, DELETE, REPLICATE, LOCK, DETACH }) // not SAVE_UPDATE
@Cascade({ ALL} )
public Collection<ROExtString> getStrings() {
return strings;
}

@NotQueryable
@OneToMany(fetch = FetchType.LAZY, mappedBy = "owner", orphanRemoval = true)
@Cascade({ PERSIST, MERGE, REMOVE, REFRESH, DELETE, REPLICATE, LOCK, DETACH }) // not SAVE_UPDATE - see [SAVE-CASCADE]
//@Cascade({ PERSIST, MERGE, REMOVE, REFRESH, DELETE, REPLICATE, LOCK, DETACH }) // not SAVE_UPDATE
@Cascade({ ALL} )
public Collection<ROExtDate> getDates() {
return dates;
}

@NotQueryable
@OneToMany(fetch = FetchType.LAZY, mappedBy = "owner", orphanRemoval = true)
@Cascade({ PERSIST, MERGE, REMOVE, REFRESH, DELETE, REPLICATE, LOCK, DETACH }) // not SAVE_UPDATE - see [SAVE-CASCADE]
//@Cascade({ PERSIST, MERGE, REMOVE, REFRESH, DELETE, REPLICATE, LOCK, DETACH }) // not SAVE_UPDATE
@Cascade({ ALL} )
public Collection<ROExtReference> getReferences() {
return references;
}

@NotQueryable
@OneToMany(fetch = FetchType.LAZY, mappedBy = "owner", orphanRemoval = true)
@Cascade({ PERSIST, MERGE, REMOVE, REFRESH, DELETE, REPLICATE, LOCK, DETACH }) // not SAVE_UPDATE - see [SAVE-CASCADE]
//@Cascade({ PERSIST, MERGE, REMOVE, REFRESH, DELETE, REPLICATE, LOCK, DETACH }) // not SAVE_UPDATE
@Cascade({ ALL} )
public Collection<ROExtPolyString> getPolys() {
return polys;
}
Expand Down
Expand Up @@ -42,9 +42,6 @@

/**
* @author lazyman
*
* Note: We do cascade = ALL, unlike in RObject class. We do not try to eliminate "get all ext values" queries like in RObject - yet.
* See [SAVE-CASCADE]
*/
@Ignore
@Entity
Expand Down
Expand Up @@ -353,7 +353,14 @@ private <T extends ObjectType> void handleObjectCommonAttributes(Class<T> type,
object.setVersion(version);

// apply modifications, ids' for new containers already filled in delta values
ItemDeltaCollectionsUtil.applyTo(modifications, prismObject);
for (ItemDelta<?, ?> modification : modifications) {
if (modification.getDefinition() == null || !modification.getDefinition().isIndexOnly()) {
modification.applyTo(prismObject);
} else {
// There's no point in applying modifications to index-only items; they are not serialized.
// (Presumably they are not indexed as well.)
}
}

handleObjectTextInfoChanges(type, modifications, prismObject, object);

Expand Down Expand Up @@ -552,11 +559,25 @@ private static <T> void markNewValuesTransientAndAddToExistingNoFetch(Collection
if (!exists) {
//noinspection unchecked
((Collection) dbCollection).add(rValue);
ctx.session.persist(rValue); // save is not cascaded to extension values any more [SAVE-CASCADE]
ctx.session.persist(rValue); // it looks that in this way we avoid SQL SELECT (at the cost of .persist that can be sometimes more costly)
}
}
}

/**
* Similar to DeltaUpdaterUtils.markNewValuesTransientAndAddToExisting but simpler.
*
* We rely on the fact that SAVE/UPDATE is now cascaded to extension items.
*/
private static <T> void markNewValuesTransientAndAddToExistingNoFetchNoPersist(Collection<? extends RAnyValue<?>> dbCollection,
Collection<PrismEntityPair<RAnyValue<?>>> newValues, Context ctx) {
for (PrismEntityPair<RAnyValue<?>> item : newValues) {
RAnyValue<?> rValue = item.getRepository();
//noinspection unchecked
((Collection) dbCollection).add(rValue);
}
}

private void deleteExtensionValues(Collection<? extends RAnyValue<?>> dbCollection,
Collection<PrismEntityPair<RAnyValue<?>>> pairsFromDelta, Context ctx) {
if (pairsFromDelta.isEmpty()) {
Expand Down Expand Up @@ -669,7 +690,7 @@ private void replaceExtensionValues(RObjectExtensionType objectExtensionType,
}

deleteFromCollectionAndDb(dbCollection, rValuesToDelete, ctx.session);
markNewValuesTransientAndAddToExistingNoFetch(dbCollection, pairsToAdd, ctx);
markNewValuesTransientAndAddToExistingNoFetchNoPersist(dbCollection, pairsToAdd, ctx);
}

private void deleteFromCollectionAndDb(Collection<? extends RAnyValue<?>> dbCollection,
Expand Down
Expand Up @@ -305,9 +305,6 @@ private <T extends ObjectType> String nonOverwriteAddObjectAttempt(PrismObject<T
updateFullObject(rObject, object);

LOGGER.trace("Saving object (non overwrite).");
// By using persist() instead of save we cascade this operation to object+assignment extension values.
// (Save is intentionally not cascaded, because we need to add+delete these values manually.)
// See [SAVE-CASCADE].
session.persist(rObject);
lookupTableHelper.addLookupTableRows(session, rObject, false);
caseHelper.addCertificationCampaignCases(session, rObject, false);
Expand Down
Expand Up @@ -226,18 +226,17 @@ private static void clearExtensionCollection(Collection<? extends RAExtBase<?>>
while (iterator.hasNext()) {
RAExtBase dbValue = iterator.next();
// we cannot filter on assignmentExtensionType because it is not present in database (yet)
session.delete(dbValue); // see [SAVE-CASCADE]
iterator.remove();
}
}

private static void clearExtensionCollection(Collection<? extends ROExtBase<?>> dbCollection, RObjectExtensionType typeToDelete,
Session session) {
Iterator<? extends ROExtBase> iterator = dbCollection.iterator();
//noinspection Java8CollectionRemoveIf
while (iterator.hasNext()) {
ROExtBase dbValue = iterator.next();
if (typeToDelete.equals(dbValue.getOwnerType())) {
session.delete(dbValue); // see [SAVE-CASCADE]
iterator.remove();
}
}
Expand Down

0 comments on commit 48ec6c7

Please sign in to comment.