Remove Guava caches, one is no longer needed - we have matchRoot now. The second case is technically incorrect as the contents of the IAtomContainer may change so it's not safe to cache here. There is a bug with AtomContainer2.add which will be fixed in another commit - for now we can use the copy constructor.

johnmay · egonw · commit 0457deac14f9 · 2022-01-01T11:32:07.000+01:00
diff --git a/base/isomorphism/src/main/java/org/openscience/cdk/isomorphism/DfState.java b/base/isomorphism/src/main/java/org/openscience/cdk/isomorphism/DfState.java
@@ -109,8 +109,7 @@ private StackFrame() {}
                                                    "has a IChemObjectBuilder set!");
         }
 
-        IAtomContainer tmp = builder.newAtomContainer();
-        tmp.add(query);
+        IAtomContainer tmp = builder.newInstance(IAtomContainer.class, query);
         this.qbonds = new IQueryBond[tmp.getBondCount()];
         this.amap = new int[query.getAtomCount()];
 
diff --git a/base/isomorphism/src/main/java/org/openscience/cdk/isomorphism/matchers/Expr.java b/base/isomorphism/src/main/java/org/openscience/cdk/isomorphism/matchers/Expr.java
@@ -23,10 +23,6 @@
 
 package org.openscience.cdk.isomorphism.matchers;
 
-import com.google.common.cache.CacheBuilder;
-import com.google.common.cache.CacheLoader;
-import com.google.common.cache.LoadingCache;
-import com.google.common.cache.Weigher;
 import org.openscience.cdk.CDKConstants;
 import org.openscience.cdk.ReactionRole;
 import org.openscience.cdk.config.Elements;
@@ -41,6 +37,7 @@
 import java.util.ArrayDeque;
 import java.util.Arrays;
 import java.util.Deque;
+import java.util.Map;
 import java.util.Objects;
 
 /**
@@ -704,9 +701,6 @@ public IAtomContainer subquery() {
         return query;
     }
 
-    /* Property Caches */
-    private static LoadingCache<IAtomContainer, int[]> cacheRCounts;
-
     private static int[] getRingCounts(IAtomContainer mol) {
         int[] rcounts = new int[mol.getAtomCount()];
         for (int[] path : Cycles.mcb(mol).paths()) {
@@ -719,24 +713,10 @@ private static int[] getRingCounts(IAtomContainer mol) {
 
     private static int getRingCount(IAtom atom) {
         final IAtomContainer mol = atom.getContainer();
-        if (cacheRCounts == null) {
-            cacheRCounts = CacheBuilder.newBuilder()
-                                       .maximumWeight(1000) // 4KB
-                                       .weigher(new Weigher<IAtomContainer, int[]>() {
-                                           @Override
-                                           public int weigh(IAtomContainer key,
-                                                            int[] value) {
-                                               return value.length;
-                                           }
-                                       })
-                                       .build(new CacheLoader<IAtomContainer, int[]>() {
-                                           @Override
-                                           public int[] load(IAtomContainer key) throws Exception {
-                                               return getRingCounts(key);
-                                           }
-                                       });
-        }
-        return cacheRCounts.getUnchecked(mol)[atom.getIndex()];
+        // we used to have a cache here but this would be incorrect if
+        // an IAtomContainer was reused - better would be if we did the
+        // matching in the pattern
+        return getRingCounts(mol)[atom.getIndex()];
     }
 
     /**
diff --git a/legacy/src/main/java/org/openscience/cdk/isomorphism/matchers/smarts/RecursiveSmartsAtom.java b/legacy/src/main/java/org/openscience/cdk/isomorphism/matchers/smarts/RecursiveSmartsAtom.java
@@ -18,17 +18,11 @@
  */
 package org.openscience.cdk.isomorphism.matchers.smarts;
 
-import com.google.common.cache.CacheBuilder;
-import com.google.common.cache.CacheLoader;
-import com.google.common.cache.LoadingCache;
 import org.openscience.cdk.interfaces.IAtom;
-import org.openscience.cdk.interfaces.IAtomContainer;
-import org.openscience.cdk.isomorphism.Pattern;
+import org.openscience.cdk.isomorphism.DfPattern;
 import org.openscience.cdk.isomorphism.matchers.IQueryAtom;
 import org.openscience.cdk.isomorphism.matchers.IQueryAtomContainer;
 
-import java.util.BitSet;
-
 /**
  * This matches recursive smarts atoms.
  *
@@ -42,8 +36,7 @@ public final class RecursiveSmartsAtom extends SMARTSAtom {
     /** The IQueryAtomContainer created by parsing the recursive smarts */
     private final IQueryAtomContainer                  query;
 
-    /** Query cache. */
-    private final LoadingCache<IAtomContainer, BitSet> cache;
+    private final DfPattern ptrn;
 
     /**
      * Creates a new instance
@@ -53,19 +46,7 @@ public final class RecursiveSmartsAtom extends SMARTSAtom {
     public RecursiveSmartsAtom(final IQueryAtomContainer query) {
         super(query.getBuilder());
         this.query = query;
-        this.cache = CacheBuilder.newBuilder().maximumSize(42).weakKeys()
-                .build(new CacheLoader<IAtomContainer, BitSet>() {
-
-                    @Override
-                    public BitSet load(IAtomContainer target) throws Exception {
-                        BitSet hits = new BitSet();
-                        for (int[] mapping : Pattern.findSubstructure(query)
-                                                    .matchAll(target)) {
-                            hits.set(mapping[0]);
-                        }
-                        return hits;
-                    }
-                });
+        this.ptrn = DfPattern.findSubstructure(query);
     }
 
     /*
@@ -76,13 +57,8 @@ public BitSet load(IAtomContainer target) throws Exception {
      */
     @Override
     public boolean matches(IAtom atom) {
-
         if (!((IQueryAtom) query.getAtom(0)).matches(atom)) return false;
-
         if (query.getAtomCount() == 1) return true;
-
-        IAtomContainer target = invariants(atom).target();
-
-        return cache.getUnchecked(target).get(target.indexOf(atom));
+        return ptrn.matchesRoot(atom);
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -109,8 +109,7 @@ private StackFrame() {}`
`109`	`109`	`"has a IChemObjectBuilder set!");`
`110`	`110`	`}`
`111`	`111`
`112`		`- IAtomContainer tmp = builder.newAtomContainer();`
`113`		`- tmp.add(query);`
	`112`	`+ IAtomContainer tmp = builder.newInstance(IAtomContainer.class, query);`
`114`	`113`	`this.qbonds = new IQueryBond[tmp.getBondCount()];`
`115`	`114`	`this.amap = new int[query.getAtomCount()];`
`116`	`115`