Skip to content

Commit

Permalink
XWIKI-22577: Cloning documents with many XObjects and high object num…
Browse files Browse the repository at this point in the history 
…bers is slow (#3596)

(cherry picked from commit f2f962c)
  • Loading branch information
@tmortagne
tmortagne committed Oct 28, 2024
1 parent 7dc47e2 commit 997a072
Show file tree
Hide file tree
Showing 3 changed files with 143 additions and 101 deletions.
97 changes: 56 additions & 41 deletions ...del/xwiki-platform-model-api/src/main/java/org/xwiki/model/reference/EntityReference.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.beans.Transient;
import java.io.Serializable;
import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -677,16 +678,6 @@ public int hashCode()
.append(this.parameters).toHashCode();
}

/**
* {@inheritDoc}
* <p>
* Note: The default implementation relies on comparing the string serialization of the 2 entities. It is the
* caller's responsibility to make sure that the entities are either first resolved or at least of the same type, in
* order for the comparison to actually make sense.
* </p>
*
* @see java.lang.Comparable#compareTo(java.lang.Object)
*/
@Override
public int compareTo(EntityReference reference)
{
Expand All @@ -698,47 +689,71 @@ public int compareTo(EntityReference reference)
return 0;
}

// Generically compare the string serializations of the 2 references.
int stringCompareResult = toString().compareTo(reference.toString());
if (stringCompareResult != 0) {
return stringCompareResult;
List<EntityReference> references = getReversedReferenceChain();
Iterator<EntityReference> it = references.iterator();
List<EntityReference> otherReferences = reference.getReversedReferenceChain();
Iterator<EntityReference> otherIt = otherReferences.iterator();
while (it.hasNext() && otherIt.hasNext()) {
EntityReference element = it.next();
EntityReference otherElement = otherIt.next();

// Compare the names
int result = element.getName().compareTo(otherElement.getName());
if (result != 0) {
return result;
}

// Compare the parameters
result = compareParameters(element.getParameters(), otherElement.getParameters());
if (result != 0) {
return result;
}
}

// If the string serializations are the same, compare the parameters.
return compareParameters(reference);
return references.size() - otherReferences.size();
}

/**
* Compare parameters of this reference and another reference.
* Compare parameters of two references.
*
* @param reference the other reference to be compare with
* @return 0 if parameters are equals, -1 if this reference has lower parameters, +1 otherwise
*/
@SuppressWarnings("unchecked")
private int compareParameters(EntityReference reference)
{
if (parameters != null && reference.parameters == null) {
return 1;
* @param parameters the first parameters to compare
* @param otherParameters the other parameters to compare
* @return 0 if parameters are equals, -1 if the first parameters are lower, +1 otherwise
*/
private int compareParameters(Map<String, Serializable> parameters, Map<String, Serializable> otherParameters)
{
for (Map.Entry<String, Serializable> entry : parameters.entrySet()) {
Object value = entry.getValue();
Object otherValue = otherParameters.get(entry.getKey());
int result = compareTo(value, otherValue);
if (result != 0) {
return result;
}
}

if (parameters != null) {
for (Map.Entry<String, Serializable> entry : parameters.entrySet()) {
Object obj = reference.parameters.get(entry.getKey());
Object myobj = entry.getValue();
if (myobj instanceof Comparable) {
if (obj == null) {
return 1;
}

int number = ((Comparable) myobj).compareTo(obj);

if (number != 0) {
return number;
}
}
return parameters.size() - otherParameters.size();
}

private int compareTo(Object value, Object otherValue)
{
if (value != otherValue) {
if (value == null) {
return -1;
}

if (otherValue == null) {
return 1;
}

if (value.getClass() == otherValue.getClass() && value instanceof Comparable) {
return ((Comparable) value).compareTo(otherValue);
}

if (!value.equals(otherValue)) {
return 1;
}
}

return (reference.parameters == null) ? 0 : -1;
return 0;
}
}
67 changes: 28 additions & 39 deletions ...i-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/doc/XWikiDocument.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import javax.inject.Provider;
import javax.servlet.http.HttpServletResponse;

import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
Expand Down Expand Up @@ -3306,7 +3307,7 @@ public void addXObject(BaseObject object)
{
object.setOwnerDocument(this);

List<BaseObject> vobj = this.xObjects.get(object.getXClassReference());
BaseObjects vobj = this.xObjects.get(object.getXClassReference());
if (vobj == null) {
setXObject(0, object);
} else {
Expand Down Expand Up @@ -3335,15 +3336,9 @@ public void setXObject(DocumentReference classReference, int nb, BaseObject obje
object.setNumber(nb);
}

BaseObjects objects = this.xObjects.get(classReference);
if (objects == null) {
objects = new BaseObjects();
this.xObjects.put(classReference, objects);
}
while (nb >= objects.size()) {
objects.add(null);
}
objects.set(nb, object);
BaseObjects objects = this.xObjects.computeIfAbsent(classReference, k -> new BaseObjects());
objects.put(nb, object);

setMetaDataDirty(true);
}

Expand All @@ -3361,15 +3356,9 @@ public void setXObject(int nb, BaseObject object)
object.setOwnerDocument(this);
object.setNumber(nb);

BaseObjects objects = this.xObjects.get(object.getXClassReference());
if (objects == null) {
objects = new BaseObjects();
this.xObjects.put(object.getXClassReference(), objects);
}
while (nb >= objects.size()) {
objects.add(null);
}
objects.set(nb, object);
BaseObjects objects = this.xObjects.computeIfAbsent(object.getXClassReference(), k -> new BaseObjects());
objects.put(nb, object);

setMetaDataDirty(true);
}

Expand Down Expand Up @@ -3473,31 +3462,32 @@ public void duplicateXObjects(XWikiDocument templatedoc)
/**
* Copy specified document objects into current document.
*
* @param templatedoc the document to copy
* @param keepsIdentity if true it does an exact java copy, otherwise it duplicate objects with the new document
* name (and new class names)
* @param templateDocument the document to copy
* @param keepsIdentity if true it does an exact copy (same guid), otherwise it create a new object with the same
* values
*/
private void cloneXObjects(XWikiDocument templatedoc, boolean keepsIdentity)
private void cloneXObjects(XWikiDocument templateDocument, boolean keepsIdentity)
{
// clean map
this.xObjects.clear();

// fill map
for (Map.Entry<DocumentReference, List<BaseObject>> entry : templatedoc.getXObjects().entrySet()) {
List<BaseObject> tobjects = entry.getValue();

// clone and insert xobjects
for (BaseObject otherObject : tobjects) {
if (otherObject != null) {
if (keepsIdentity) {
addXObject(otherObject.clone());
} else {
BaseObject newObject = otherObject.duplicate(getDocumentReference());
setXObject(newObject.getNumber(), newObject);
for (Map.Entry<DocumentReference, BaseObjects> entry : templateDocument.xObjects.entrySet()) {
BaseObjects tobjects = entry.getValue();

if (CollectionUtils.isNotEmpty(tobjects)) {
BaseObjects objects = new BaseObjects(this, tobjects, keepsIdentity);

if (!objects.isEmpty()) {
DocumentReference xclassReference = entry.getKey();
WikiReference wikiReference = getDocumentReference().getWikiReference();
if (!wikiReference.equals(xclassReference.getWikiReference())) {
// Make sure the class reference is in the same wiki as the document in which the object is
// stored
xclassReference = xclassReference.setWikiReference(wikiReference);
}
} else if (keepsIdentity) {
// set null object to make sure to have exactly the same thing when cloning a document
addXObject(entry.getKey(), null);

this.xObjects.put(xclassReference, objects);
}
}
}
Expand Down Expand Up @@ -4597,13 +4587,12 @@ private XWikiDocument cloneInternal(DocumentReference newDocumentReference, bool

if (keepsIdentity) {
doc.setXClassXML(getXClassXML());
doc.cloneXObjects(this);
doc.cloneAttachments(this);
} else {
doc.getXClass().setCustomMapping(null);
doc.duplicateXObjects(this);
doc.copyAttachments(this);
}
doc.cloneXObjects(this, keepsIdentity);

doc.setContentDirty(isContentDirty());
doc.setMetaDataDirty(isMetaDataDirty());
Expand Down
80 changes: 59 additions & 21 deletions ...orm-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/doc/BaseObjects.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.Map;
import java.util.concurrent.ConcurrentSkipListMap;

import com.xpn.xwiki.doc.XWikiDocument;
import com.xpn.xwiki.objects.BaseObject;

/**
Expand Down Expand Up @@ -61,6 +62,38 @@ public BaseObjects(Collection<BaseObject> collection)
collection.forEach(this::add);
}

/**
* @param document the document when the current {@link BaseObjects} is stored
* @param otherObjects the objects to copy
* @param keepsIdentity if true it does an exact copy (same guid), otherwise it create a new object with the same
* values
* @since 16.10.0RC1
* @since 16.4.5
* @since 15.10.14
*/
public BaseObjects(XWikiDocument document, BaseObjects otherObjects, boolean keepsIdentity)
{
// Make sure to keep the same size if keepsIdentity is true
this.size = keepsIdentity ? otherObjects.size() : 0;

for (Map.Entry<Integer, BaseObject> entry : otherObjects.map.entrySet()) {
BaseObject otherObject = entry.getValue();
BaseObject clonedObject =
keepsIdentity ? otherObject.clone() : otherObject.duplicate(document.getDocumentReference());

// Make sure the cloned object has the right document (in case was is cloned in the document)
clonedObject.setOwnerDocument(document);

// Make sure to maintain the same position in the list since it's part of the object reference
put(entry.getKey(), clonedObject);

// Update the size
if (this.size <= otherObject.getNumber()) {
this.size = otherObject.getNumber() + 1;
}
}
}

@Override
public BaseObject get(int index)
{
Expand All @@ -75,27 +108,6 @@ public int size()
return this.size;
}

private BaseObject put(int index, BaseObject element)
{
BaseObject old;
if (element == null) {
// We don't want to keep null values in memory
old = this.map.remove(index);
} else {
// Make sure the object number is right
element.setNumber(index);

old = this.map.put(index, element);
}

// Increment size if needed
if (this.size <= index) {
this.size = index + 1;
}

return old;
}

@Override
public void add(int index, BaseObject element)
{
Expand Down Expand Up @@ -123,6 +135,32 @@ public BaseObject set(int index, BaseObject element)
return put(index, element);
}

/**
* @param index the index of the object in the list
* @param object the object
* @return the object previously at the specified position
*/
public BaseObject put(int index, BaseObject object)
{
BaseObject old;
if (object == null) {
// We don't want to keep null values in memory
old = this.map.remove(index);
} else {
// Make sure the object number is right
object.setNumber(index);

old = this.map.put(index, object);
}

// Increment size if needed
if (this.size <= index) {
this.size = index + 1;
}

return old;
}

@Override
public BaseObject remove(int index)
{
Expand Down

0 comments on commit 997a072

Please sign in to comment.