Improve recursive comparison performance by caching field and field n…

…ames result.

Document performance cost of ignoring collection order
Fix #2979

assertj-core/src/main/java/org/assertj/core/api/RecursiveComparisonAssert.java

@@ -1144,6 +1144,10 @@ public SELF ignoringOverriddenEqualsForFieldsMatchingRegexes(String... regexes)
   /**
    * Makes the recursive comparison to ignore collection order in all fields in the object under test.
    * <p>
+   * <b>Important:</b> ignoring collection order has a high performance cost because each element of the actual collection must
+   * be compared to each element of the expected collection which is a O(n&sup2;) operation. For example with a collection of 100
+   * elements, the number of comparisons is 100x100 = 10 000!
+   * <p>
    * Example:
    * <pre><code class='java'> class Person {
    *   String name;
@@ -1178,6 +1182,10 @@ public SELF ignoringCollectionOrder() {
   /**
    * Makes the recursive comparison to ignore collection order in the object under test specified fields. Nested fields can be specified like this: {@code home.address.street}.
    * <p>
+   * <b>Important:</b> ignoring collection order has a high performance cost because each element of the actual collection must
+   * be compared to each element of the expected collection which is a O(n&sup2;) operation. For example with a collection of 100
+   * elements, the number of comparisons is 100x100 = 10 000!
+   * <p>
    * Example:
    * <pre><code class='java'> class Person {
    *   String name;
@@ -1222,6 +1230,10 @@ public SELF ignoringCollectionOrderInFields(String... fieldsToIgnoreCollectionOr
    * Nested fields can be specified by using dots like this: {@code home\.address\.street} ({@code \} is used to escape
    * dots since they have a special meaning in regexes).
    * <p>
+   * <b>Important:</b> ignoring collection order has a high performance cost because each element of the actual collection must
+   * be compared to each element of the expected collection which is a O(n&sup2;) operation. For example with a collection of 100
+   * elements, the number of comparisons is 100x100 = 10 000!
+   * <p>
    * Example:
    * <pre><code class='java'> class Person {
    *   String name;

assertj-core/src/main/java/org/assertj/core/api/recursive/comparison/DefaultRecursiveComparisonIntrospectionStrategy.java

@@ -12,12 +12,14 @@
  */
 package org.assertj.core.api.recursive.comparison;
 
+import static org.assertj.core.internal.Objects.getFieldsNames;
 import static org.assertj.core.util.introspection.PropertyOrFieldSupport.COMPARISON;
 
 import java.util.HashSet;
+import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 
-import org.assertj.core.internal.Objects;
 import org.assertj.core.util.introspection.PropertyOrFieldSupport;
 
 /**
@@ -28,9 +30,17 @@
  */
 public class DefaultRecursiveComparisonIntrospectionStrategy implements RecursiveComparisonIntrospectionStrategy {
 
+  // use ConcurrentHashMap in case this strategy instance is used in a multi-thread context
+  private final Map<Class<?>, Set<String>> fieldsNamesPerClass = new ConcurrentHashMap<>();
+
   @Override
   public Set<String> getChildrenNodeNamesOf(Object node) {
-    return node == null ? new HashSet<>() : Objects.getFieldsNames(node.getClass());
+    if (node == null) return new HashSet<>();
+    // Caches the node names after getting them for efficiency, a node can be introspected multiple times if for example
+    // it belongs to an unordered collection as all actual elements are compared to all expected elements.
+    Class<?> nodeClass = node.getClass();
+    fieldsNamesPerClass.computeIfAbsent(nodeClass, unused -> getFieldsNames(nodeClass));
+    return fieldsNamesPerClass.get(nodeClass);
   }
 
   @Override

assertj-core/src/main/java/org/assertj/core/api/recursive/comparison/RecursiveComparisonConfiguration.java

@@ -322,6 +322,10 @@ boolean getIgnoreCollectionOrder() {
   /**
    * Sets whether to ignore collection order in the comparison.
    * <p>
+   * <b>Important:</b> ignoring collection order has a high performance cost because each element of the actual collection must
+   * be compared to each element of the expected collection which is a O(n&sup2;) operation. For example with a collection of 100
+   * elements, the number of comparisons is 100x100 = 10 000!
+   * <p>
    * See {@link RecursiveComparisonAssert#ignoringCollectionOrder()} for code examples.
    *
    * @param ignoreCollectionOrder whether to ignore collection order in the comparison.
@@ -333,6 +337,10 @@ public void ignoreCollectionOrder(boolean ignoreCollectionOrder) {
   /**
    * Adds the given fields to the list fields from the object under test to ignore collection order in the recursive comparison.
    * <p>
+   * <b>Important:</b> ignoring collection order has a high performance cost because each element of the actual collection must
+   * be compared to each element of the expected collection which is a O(n&sup2;) operation. For example with a collection of 100
+   * elements, the number of comparisons is 100x100 = 10 000!
+   * <p>
    * See {@link RecursiveComparisonAssert#ignoringCollectionOrderInFields(String...) RecursiveComparisonAssert#ignoringCollectionOrderInFields(String...)} for examples.
    *
    * @param fieldsToIgnoreCollectionOrder the fields of the object under test to ignore collection order in the comparison.

assertj-core/src/main/java/org/assertj/core/util/introspection/FieldUtils.java

@@ -17,6 +17,8 @@
 
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * Shameless copy from Apache commons lang and then modified to keep only the interesting stuff for AssertJ.
@@ -29,6 +31,9 @@
  */
 class FieldUtils {
 
+  // use ConcurrentHashMap as FieldUtils can be used in a multi-thread context
+  private static final Map<Class<?>, Map<String, Field>> fieldsPerClass = new ConcurrentHashMap<>();
+
   /**
    * Gets an accessible <code>Field</code> by name breaking scope if requested. Superclasses/interfaces will be
    * considered.
@@ -60,7 +65,7 @@ static Field getField(final Class<?> cls, String fieldName, boolean forceAccess)
     // check up the superclass hierarchy
     for (Class<?> acls = cls; acls != null; acls = acls.getSuperclass()) {
       try {
-        Field field = acls.getDeclaredField(fieldName);
+        Field field = getDeclaredField(fieldName, acls);
         // getDeclaredField checks for non-public scopes as well and it returns accurate results
         if (!Modifier.isPublic(field.getModifiers())) {
           if (forceAccess) {
@@ -91,6 +96,25 @@ static Field getField(final Class<?> cls, String fieldName, boolean forceAccess)
     return match;
   }
 
+  /**
+   * Returns the {@link Field} corresponding to the given fieldName for the specified class.
+   * <p>
+   * Caches the field after getting it for efficiency.
+   *
+   * @param fieldName the name of the field to get
+   * @param acls the class to introspect
+   * @return the {@link Field} corresponding to the given fieldName for the specified class.
+   * @throws NoSuchFieldException bubbled up from the call to {@link Class#getDeclaredField(String)}
+   */
+  private static Field getDeclaredField(String fieldName, Class<?> acls) throws NoSuchFieldException {
+    fieldsPerClass.computeIfAbsent(acls, unused -> new ConcurrentHashMap<>());
+    // can't use computeIfAbsent for fieldName as getDeclaredField throws a checked exception
+    if (fieldsPerClass.get(acls).containsKey(fieldName)) return fieldsPerClass.get(acls).get(fieldName);
+    Field field = acls.getDeclaredField(fieldName);
+    fieldsPerClass.get(acls).put(fieldName, field);
+    return acls.getDeclaredField(fieldName);
+  }
+
   /**
    * Reads an accessible Field.
    *

assertj-core/src/test/java/org/assertj/core/api/recursive/comparison/RecursiveComparisonAssert_isEqualTo_Test.java

@@ -13,6 +13,7 @@
 package org.assertj.core.api.recursive.comparison;
 
 import static java.lang.String.format;
+import static java.util.stream.Collectors.toList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.BDDAssertions.entry;
 import static org.assertj.core.api.BDDAssertions.then;
@@ -37,6 +38,7 @@
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.IntStream;
 import java.util.stream.Stream;
 
 import javax.xml.datatype.DatatypeFactory;
@@ -48,6 +50,7 @@
 import org.assertj.core.internal.objects.data.Human;
 import org.assertj.core.internal.objects.data.Person;
 import org.assertj.core.util.DoubleComparator;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
@@ -548,28 +551,85 @@ public EmployeeDTO(String name, JobTitle jobTitle) {
       this.jobTitle = jobTitle;
     }
 
-    public enum JobTitle
-    {
+    public enum JobTitle {
       SOFTWARE_DEVELOPER, QA_ENGINEER
     }
   }
 
   // https://github.com/assertj/assertj/issues/2928
   @ParameterizedTest(name = "class: {2}")
   @MethodSource
-  void should_not_introspect_java_base_classes(Object actual, Object expected, @SuppressWarnings("unused")  String testDescription) {
+  void should_not_introspect_java_base_classes(Object actual, Object expected,
+                                               @SuppressWarnings("unused") String testDescription) {
     assertThat(actual).usingRecursiveComparison()
                       .isEqualTo(expected);
   }
 
   private static Stream<Arguments> should_not_introspect_java_base_classes() throws Exception {
 
-    return Stream.of(arguments(DatatypeFactory.newInstance().newXMLGregorianCalendar(), 
-                               DatatypeFactory.newInstance().newXMLGregorianCalendar(), 
+    return Stream.of(arguments(DatatypeFactory.newInstance().newXMLGregorianCalendar(),
+                               DatatypeFactory.newInstance().newXMLGregorianCalendar(),
                                "com.sun.org.apache.xerces.internal.jaxp.datatype.XMLGregorianCalendarImpl"),
                      arguments(InetAddress.getByName("127.0.0.1"),
                                InetAddress.getByName("127.0.0.1"),
                                InetAddress.class.getName()));
   }
 
+  // https://github.com/assertj/assertj/issues/2979
+
+  static class Assignment {
+    String assignmentName;
+    String assignmentDescription;
+
+    Assignment(String assignmentName, String assignmentDescription) {
+      this.assignmentName = assignmentName;
+      this.assignmentDescription = assignmentDescription;
+    }
+  }
+
+  static class Owner {
+    String id;
+    String name;
+    List<Assignment> assignments;
+
+    Owner(String id, String name, List<Assignment> assignments) {
+      this.id = id;
+      this.name = name;
+      this.assignments = assignments;
+    }
+  }
+
+  public static List<Assignment> generateListOfAssignments(int size, String id) {
+    return IntStream.range(0, size)
+                    .mapToObj(i -> new Assignment(id, "Assignment " + id))
+                    .collect(toList());
+  }
+
+  public static List<Owner> generateListOfOwners(int noOfOwners, int noOfAssignments, String id) {
+    String ownerId = "owner" + id;
+    String ownerName = "TestOwner " + id;
+    return IntStream.range(0, noOfOwners)
+                    .mapToObj(i -> new Owner(ownerId, ownerName, generateListOfAssignments(noOfAssignments, id)))
+                    .collect(toList());
+  }
+
+  @Test
+  @Disabled("just to check performance of the recursive comparison when using big collections")
+  void performance_for_comparing_lots_of_similar_types() {
+    // 100 owners with 50 assignments each
+    List<Owner> actual = generateListOfOwners(50, 50, "1"); // 2500 assignment
+
+    // 100 different owners with 50 different assignments each
+    List<Owner> expected = generateListOfOwners(50, 50, "2"); // 2500 assignment
+
+    // Recursive comparison logic
+    RecursiveComparisonConfiguration config = new RecursiveComparisonConfiguration();
+    // when uncommented, test is slow as we compare recursively 2500x2500 elements
+    config.ignoreCollectionOrder(true);
+    // config.ignoreCollectionOrderInFields();
+    // config.setIntrospectionStrategy(ComparingFields.COMPARING_FIELDS);
+
+    new RecursiveComparisonDifferenceCalculator().determineDifferences(actual, expected, config);
+
+  }
 }