Merge pull request #1 from schlosna/aash/usingRecursiveComparison-slo…

…wness-repro

Optimize FieldLocation

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

@@ -14,14 +14,17 @@
 
 import static java.util.Collections.emptyList;
 import static java.util.Collections.unmodifiableList;
+import static java.util.Collections.unmodifiableSet;
 import static java.util.Objects.requireNonNull;
 import static java.util.stream.Collectors.joining;
-import static java.util.stream.Collectors.toList;
 import static org.assertj.core.util.Lists.list;
+import static org.assertj.core.util.Sets.newLinkedHashSet;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
+import java.util.Set;
 import java.util.regex.Pattern;
 
 /**
@@ -33,7 +36,7 @@ public final class FieldLocation implements Comparable<FieldLocation> {
 
   private final String pathToUseInRules;
   private final List<String> decomposedPath;
-  private final List<String> pathsHierarchyToUseInRules;
+  private final Set<String> pathsHierarchyToUseInRules;
 
   public FieldLocation(List<String> path) {
     decomposedPath = unmodifiableList(requireNonNull(path, "path cannot be null"));
@@ -163,7 +166,10 @@ public boolean equals(Object obj) {
 
   @Override
   public int hashCode() {
-    return Objects.hash(pathToUseInRules, decomposedPath, pathsHierarchyToUseInRules);
+    int result = Objects.hashCode(pathToUseInRules);
+    result = 31 * result + Objects.hashCode(decomposedPath);
+    result = 31 * result + Objects.hashCode(pathsHierarchyToUseInRules);
+    return result;
   }
 
   @Override
@@ -195,6 +201,10 @@ public String getFieldName() {
   public boolean isRoot() {
     // root is the top level object compared or in case of the top level is a iterable/array the elements are considered as roots.
     // we don't do it for optional it has a 'value' field so for the moment
+    return isRootPath(pathToUseInRules);
+  }
+
+  private boolean isRootPath(String pathToUseInRules) {
     return pathToUseInRules.isEmpty();
   }
 
@@ -256,21 +266,23 @@ public boolean hasChild(FieldLocation child) {
     return child.hasParent(this);
   }
 
-  private List<String> pathsHierarchyToUseInRules() {
-    List<FieldLocation> fieldAndParentFields = list();
-    FieldLocation currentLocation = this;
-    while (!currentLocation.isRoot()) {
-      fieldAndParentFields.add(currentLocation);
-      currentLocation = currentLocation.parent();
+  private Set<String> pathsHierarchyToUseInRules() {
+    // using LinkedHashSet to maintain leaf to root iteration order
+    // so that hierarchyMatchesRegex can try matching longest to shorted path
+    Set<String> fieldAndParentFields = newLinkedHashSet();
+    String currentPath = this.pathToUseInRules;
+    while (!isRootPath(currentPath)) {
+      fieldAndParentFields.add(currentPath);
+      currentPath = parent(currentPath);
     }
-    return fieldAndParentFields.stream()
-                               .map(fieldLocation -> fieldLocation.pathToUseInRules)
-                               .collect(toList());
+    return unmodifiableSet(fieldAndParentFields);
   }
 
-  private FieldLocation parent() {
-    List<String> parentPath = new ArrayList<>(decomposedPath);
-    parentPath.remove(decomposedPath.size() - 1);
-    return new FieldLocation(parentPath);
+  private String parent(String currentPath) {
+    int lastDot = currentPath.lastIndexOf('.');
+    if (lastDot < 0) {
+      return "";
+    }
+    return currentPath.substring(0, lastDot);
   }
 }

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

@@ -635,26 +635,26 @@ void can_compare_deeply_nested_objects_in_reasonable_time() {
     p19a.neighbour = p20a;
     p19b.neighbour = p20b;
 
-    // This fails at 15sec > 10sec on my 2021 Apple M1 Pro.  Uncomment more references below to increase the time. Every
+    // This fails at 15sec > 10sec on my 2021 Apple M1 Pro. Uncomment more references below to increase the time. Every
     // additional link roughly doubles the execution time.
 
-//    p20a.neighbour = p21a;
-//    p20b.neighbour = p21b;
+    p20a.neighbour = p21a;
+    p20b.neighbour = p21b;
 
-//    p21a.neighbour = p22a;
-//    p21b.neighbour = p22b;
+    p21a.neighbour = p22a;
+    p21b.neighbour = p22b;
 
-//    p22a.neighbour = p23a;
-//    p22b.neighbour = p23b;
+    p22a.neighbour = p23a;
+    p22b.neighbour = p23b;
 
-//    p23a.neighbour = p24a;
-//    p23b.neighbour = p24b;
+    p23a.neighbour = p24a;
+    p23b.neighbour = p24b;
 
-//    p24a.neighbour = p25a;
-//    p24b.neighbour = p25b;
+    p24a.neighbour = p25a;
+    p24b.neighbour = p25b;
 
-//    p25a.neighbour = p26a;
-//    p25b.neighbour = p26b;
+    p25a.neighbour = p26a;
+    p25b.neighbour = p26b;
 
     Stopwatch stopwatch = Stopwatch.createStarted();
     assertThat(p1a).usingRecursiveComparison().isEqualTo(p1b);