Final clean-up

- Removed unused method from earlier version
- Corrected previously overlooked comments that were remnant of prior version

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/DDCachingPoolStrategy.java

@@ -22,22 +22,15 @@ import net.bytebuddy.pool.TypePool;
  *
  * <ul>
  *   There two core parts to the cache...
- *   <li>a cache of ID assignments for ClassLoaders
+ *   <li>a cache of ClassLoader to WeakReference&lt;ClassLoader&gt;
- *   <li>a single cache of TypeResolutions for all ClassLoaders - keyed by a custom composite key
+ *   <li>a single cache of TypeResolutions for all ClassLoaders - keyed by a custom composite key of ClassLoader & class name
- *       that combines loader ID & name
  * </ul>
  *
  * <p>This design was chosen to create a single limited size cache that can be adjusted for the
  * entire application -- without having to create a large number of WeakReference objects.
  *
- * <p>The ID assignment mostly assigns a single ID to each ClassLoader, but the maximumSize
+ * <p>Eviction is handled almost entirely through a size restriction; however,
- * restriction means that an evicted ClassLoader could be assigned another ID later on.
+ * softValues are still used as a further safeguard.
- *
- * <p>For the validity of the cache, the important part is that ID assignment guarantees that no two
- * ClassLoaders share the same ID.
- *
- * <p>NOTE: As an additional safe-guard, a new CacheInstance can be created if the original loader
- * ID sequence is exhausted.
  */
 @Slf4j
 public class DDCachingPoolStrategy implements PoolStrategy {
@@ -99,11 +92,6 @@ public class DDCachingPoolStrategy implements PoolStrategy {
     return createCachingTypePool(loaderHash, loaderRef, classFileLocator);
   }
 
-  private final TypePool createNonCachingTypePool(final ClassFileLocator classFileLocator) {
-    return new TypePool.Default.WithLazyResolution(
-        TypePool.CacheProvider.NoOp.INSTANCE, classFileLocator, TypePool.Default.ReaderMode.FAST);
-  }
-
   private final TypePool.CacheProvider createCacheProvider(
       final int loaderHash, final WeakReference<ClassLoader> loaderRef) {
     return new SharedResolutionCacheAdapter(loaderHash, loaderRef, sharedResolutionCache);
@@ -130,8 +118,13 @@ public class DDCachingPoolStrategy implements PoolStrategy {
   }
 
   /**
-   * TypeCacheKey is key for the sharedResolutionCache. It is a mix of a cacheId/loaderId & a type
+   * TypeCacheKey is key for the sharedResolutionCache.
-   * name.
+   * Conceptually, it is a mix of ClassLoader & class name.
+   *
+   * For efficiency & GC purposes, it is actually composed of
+   * loaderHash & WeakReference&lt;ClassLoader&gt;
+   *
+   * The loaderHash exists to avoid calling get & strengthening the Reference.
    */
   static final class TypeCacheKey {
     private final int loaderHash;
@@ -169,11 +162,15 @@ public class DDCachingPoolStrategy implements PoolStrategy {
         return className.equals(that.className);
       } else if (className.equals(that.className)) {
         // need to perform a deeper loader check -- requires calling Reference.get
-        // which can strengthened the Reference, so deliberately done last
+        // which can strengthen the Reference, so deliberately done last
 
         // If either reference has gone null, they aren't considered equivalent
         // Technically, this is a bit of violation of equals semantics, since
-        // two equivalent references can be not equivalent.
+        // two equivalent references can become not equivalent.
+
+        // In this case, it is fine because that means the ClassLoader is no
+        // longer live, so the entries will never match anyway and will fall
+        // out of the cache.
         ClassLoader thisLoader = loaderRef.get();
         if (thisLoader == null) return false;