Memory leak patch and performance benchmarking

lib/src/main/java/badgerlog/events/EventRegistry.java

@@ -6,20 +6,19 @@
 import edu.wpi.first.networktables.NetworkTableInstance;
 import edu.wpi.first.wpilibj.Timer;
 
-import java.util.ArrayList;
 import java.util.EnumSet;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Queue;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentLinkedQueue;
 
 /**
  * Contains all instances of events and a queue for events to be synchronized on the main thread
  */
 public class EventRegistry {
     private static final Queue<WatcherPair<?>> eventQueue = new ConcurrentLinkedQueue<>();
-    private static final Map<String, List<NTEntry<?>>> watchedEntries = new HashMap<>();
+    private static final Map<String, Queue<NTEntry<?>>> watchedEntries = new ConcurrentHashMap<>();
 
     /**
      * Activates any queued events from the previous loop
@@ -74,14 +73,14 @@ public static void registerWatcher(WatcherEvent<?> event, EventMetadata metadata
      */
     public static void addWatchedEntry(NTEntry<?> entry, List<String> watcherNames) {
         for (String watcher : watcherNames) {
-            List<NTEntry<?>> entries = watchedEntries.computeIfAbsent(watcher, k -> new ArrayList<>());
+            Queue<NTEntry<?>> entries = watchedEntries.computeIfAbsent(watcher, k -> new ConcurrentLinkedQueue<>());
             entries.add(entry);
         }
     }
 
     @SuppressWarnings("unchecked")
     private static void addManagedWatcherEvent(WatcherEvent<?> event, EventMetadata metadata, NetworkTableEvent ntEvent) {
-        List<NTEntry<?>> namedEntries = watchedEntries.getOrDefault(metadata.name(), new ArrayList<>());
+        Queue<NTEntry<?>> namedEntries = watchedEntries.getOrDefault(metadata.name(), new ConcurrentLinkedQueue<>());
 
         for (NTEntry<?> entry : namedEntries) {
             String actualKey = "/BadgerLog/" + entry.getKey();

lib/src/main/java/badgerlog/processing/EntryAspect.java

@@ -32,15 +32,18 @@
 import java.lang.reflect.Member;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
+import java.util.List;
 
 /**
  * Utilizes AspectJ to weave entry generation and management into target classes.
  */
 @Aspect
 public class EntryAspect {
     private final Entries entries = new Entries(new HashMap<>());
+    private final List<Class<?>> blacklistedClasses = new ArrayList<>();
 
     @Pointcut("!within(edu.wpi.first..*) && !within(badgerlog..*) && !within(java..*) && !within(javax..*)")
     public void onlyRobotCode() {
@@ -73,6 +76,13 @@ public void getterMethodExecution(Entry entry) {
     @After("onlyRobotCode() && staticinitialization(*)")
     public void createStaticEntries(JoinPoint joinPoint) {
         Class<?> clazz = joinPoint.getSignature().getDeclaringType();
+        if (blacklistedClasses.contains(clazz)) {
+            return;
+        }
+        if (!Members.hasAnyOfAnnotation(clazz, Entry.class)) {
+            blacklistedClasses.add(clazz);
+        }
+
         entries.addInstance(clazz, null);
 
         Members.iterateOverAnnotatedFields(clazz, Entry.class, true, field -> createFieldEntry(field, null));
@@ -83,16 +93,25 @@ public void createStaticEntries(JoinPoint joinPoint) {
     @After("onlyRobotCode() && newInitialization()")
     public void createInstanceEntries(JoinPoint joinPoint) {
         Object instance = joinPoint.getThis();
-        entries.addInstance(instance.getClass(), instance);
+        Class<?> clazz = joinPoint.getSignature().getDeclaringType();
+
+        if (blacklistedClasses.contains(clazz)) {
+            return;
+        }
+        if (!Members.hasAnyOfAnnotation(clazz, Entry.class)) {
+            blacklistedClasses.add(clazz);
+        }
+
+        entries.addInstance(clazz, instance);
 
         Members.iterateOverAnnotatedFields(instance
                 .getClass(), Entry.class, false, field -> createFieldEntry(field, instance));
 
         Members.iterateOverAnnotatedMethods(instance
                 .getClass(), Entry.class, false, method -> createMethodEntry(method, instance));
 
-        if (instance.getClass().isAnnotationPresent(Entry.class)) {
-            Field[] allFields = instance.getClass().getFields();
+        if (clazz.isAnnotationPresent(Entry.class)) {
+            Field[] allFields = clazz.getFields();
             Arrays.stream(allFields)
                     .filter(this::isValidForClassGeneration)
                     .forEach(field -> createFieldEntry(field, instance));

lib/src/main/java/badgerlog/processing/EventAspect.java

@@ -14,6 +14,8 @@
 import org.aspectj.lang.annotation.Pointcut;
 
 import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 
 /**
@@ -22,6 +24,8 @@
 @Aspect
 public class EventAspect {
 
+    private final List<Class<?>> blacklistedClasses = new ArrayList<>();
+
     @Pointcut("!within(edu.wpi.first..*) && !within(badgerlog..*) && !within(java..*) && !within(javax..*)")
     public void onlyRobotCode() {
     }
@@ -34,6 +38,13 @@ public void newInitialization() {
     public void createStaticEvents(JoinPoint joinPoint) {
         Class<?> clazz = joinPoint.getSignature().getDeclaringType();
 
+        if (blacklistedClasses.contains(clazz)) {
+            return;
+        }
+        if (!Members.hasAnyOfAnnotation(clazz, Watcher.class) && !Members.hasAnyOfAnnotation(clazz, RawWatcher.class)) {
+            blacklistedClasses.add(clazz);
+        }
+
         Members.iterateOverAnnotatedMethods(clazz, Watcher.class, true, method -> handleWatcherMethod(method, null));
         Members.iterateOverAnnotatedMethods(clazz, RawWatcher.class, true, method -> handleRawWatcherMethod(method, null));
     }
@@ -43,6 +54,13 @@ public void createInstanceEvents(JoinPoint joinPoint) {
         Class<?> clazz = joinPoint.getSignature().getDeclaringType();
         Object workingClass = joinPoint.getThis();
 
+        if (blacklistedClasses.contains(clazz)) {
+            return;
+        }
+        if (!Members.hasAnyOfAnnotation(clazz, Watcher.class) && !Members.hasAnyOfAnnotation(clazz, RawWatcher.class)) {
+            blacklistedClasses.add(clazz);
+        }
+
         Members.iterateOverAnnotatedMethods(clazz, Watcher.class, false, method -> handleWatcherMethod(method, workingClass));
         Members.iterateOverAnnotatedMethods(clazz, RawWatcher.class, false, method -> handleRawWatcherMethod(method, workingClass));
     }

lib/src/main/java/badgerlog/processing/data/ClassData.java

@@ -1,20 +1,29 @@
 package badgerlog.processing.data;
 
+import lombok.Getter;
+
 import java.lang.reflect.Field;
 import java.util.Map;
 import java.util.Objects;
 
 /**
  * Holds the data used for an entire class with entries inside. Contains all instances of the class.
  */
-public record ClassData(Map<String, Field> fieldMap, Map<Object, InstanceData> instanceEntries) {
+public final class ClassData {
+    private final Map<String, Field> fieldMap;
+    private final Map<Object, InstanceData> instanceEntries;
+
+    @Getter
+    private int instanceCount = 0;
     /**
      * Constructs a new ClassData given two maps for fields and instances.
      *
      * @param fieldMap the map of strings to fields to use
      * @param instanceEntries the map of instances to {@link InstanceData}
      */
-    public ClassData {
+    public ClassData(Map<String, Field> fieldMap, Map<Object, InstanceData> instanceEntries) {
+        this.fieldMap = fieldMap;
+        this.instanceEntries = instanceEntries;
     }
 
     /**
@@ -26,10 +35,38 @@ public void addField(Field field) {
         fieldMap.put(field.getName(), field);
     }
 
-    /**
-     * {@return the number of instances currently present for the class}
-     */
-    public int getInstanceCount() {
-        return Math.toIntExact(instanceEntries().keySet().stream().filter(Objects::nonNull).count());
+    public Map<String, Field> fieldMap() {
+        return fieldMap;
+    }
+
+    public Map<Object, InstanceData> instanceEntries() {
+        return instanceEntries;
+    }
+
+    public void incrementInstanceCount() {
+        instanceCount++;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (obj == this) {
+            return true;
+        }
+        if (obj == null || obj.getClass() != this.getClass()) {
+            return false;
+        }
+        var that = (ClassData) obj;
+        return Objects.equals(this.fieldMap, that.fieldMap) && Objects
+                .equals(this.instanceEntries, that.instanceEntries);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(fieldMap, instanceEntries);
+    }
+
+    @Override
+    public String toString() {
+        return "ClassData[" + "fieldMap=" + fieldMap + ", " + "instanceEntries=" + instanceEntries + ']';
     }
 }

lib/src/main/java/badgerlog/processing/data/Entries.java

@@ -3,6 +3,7 @@
 import java.lang.reflect.Field;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.WeakHashMap;
 
 /**
  * Contains all the generated entry data.
@@ -58,10 +59,12 @@ public InstanceData getInstanceEntries(Class<?> clazz, Object instance) {
     public void addInstance(Class<?> clazz, Object instance) {
         ClassData classData = getClassData(clazz);
         if (classData == null) {
-            classData = new ClassData(new HashMap<>(), new HashMap<>());
+            classData = new ClassData(new HashMap<>(), new WeakHashMap<>());
             classDataMap.put(clazz, classData);
         }
-
+        if (instance != null) {
+            classData.incrementInstanceCount();
+        }
         classData.instanceEntries().putIfAbsent(instance, new InstanceData(new HashMap<>()));
     }
 

lib/src/main/java/badgerlog/utilities/CheckedNetworkTablesMap.java

@@ -1,5 +1,6 @@
 package badgerlog.utilities;
 
+import badgerlog.networktables.MockNTEntry;
 import badgerlog.networktables.NT;
 import badgerlog.networktables.NTEntry;
 import badgerlog.networktables.NTUpdatable;
@@ -24,8 +25,20 @@ public NT put(String key, NT value) {
         if (containsKey(key)) {
             NT oldValue = this.get(key);
             if (oldValue instanceof AutoCloseable closeable) {
+                String extraInfo = "";
+
+                if (oldValue instanceof NTEntry<?> entry) {
+                    extraInfo = entry.getKey();
+                }
+                if (oldValue instanceof MockNTEntry entry) {
+                    extraInfo = entry.realEntry().getKey();
+                }
+
+                extraInfo = extraInfo.isEmpty() ? "No Extra Info" : extraInfo + " closed by " + key;
+                extraInfo += "\n";
+
                 closeable.close();
-                ErrorLogger.customError("NetworkTable entry closed from adding another entry");
+                ErrorLogger.customError("NetworkTable entry closed from adding another entry.\n" + extraInfo);
             }
         }
 

lib/src/main/java/badgerlog/utilities/Members.java

@@ -48,6 +48,22 @@ public static void iterateOverAnnotatedFields(Class<?> clazz, Class<? extends An
                 .forEach(consumer);
     }
 
+    /**
+     * Checks for any occurrences of an annotation in a specific class. Checks the class level, field level, and method
+     * level annotations
+     *
+     * @param clazz the class to search in
+     * @param annotationClass the annotation to search for
+     *
+     * @return whether the class has any instances of the annotation
+     */
+    public static boolean hasAnyOfAnnotation(Class<?> clazz, Class<? extends Annotation> annotationClass) {
+        Member[] fields = getFieldsWithAnnotation(clazz, annotationClass);
+        Member[] methods = getMethodsWithAnnotation(clazz, annotationClass);
+        boolean hasClassAnnotation = clazz.isAnnotationPresent(annotationClass);
+        return fields.length > 0 || methods.length > 0 || hasClassAnnotation;
+    }
+
     /**
      * {@code object} defaults to {@code null}. This method should only be used if the field is known to be static.
      *

test-project/build.gradle

@@ -6,6 +6,7 @@ plugins {
     id "idea"
 //    id 'io.freefair.aspectj' version '8.14'
     id 'io.freefair.aspectj.post-compile-weaving' version '8.14'
+    id 'me.champeau.jmh' version '0.7.2'
 }
 
 def javaVersion = JavaVersion.VERSION_17
@@ -86,11 +87,24 @@ dependencies {
     testImplementation "org.junit.jupiter:junit-jupiter-params"
     testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine"
 
+    jmh 'org.openjdk.jmh:jmh-core:1.37'
+    jmhAnnotationProcessor 'org.openjdk.jmh:jmh-generator-annprocess:1.37'
+
     implementation "org.aspectj:aspectjrt:1.9.24"
-    aspect files("../lib/build/libs/lib.jar")
+    aspect project(":badgerlog")
+
+    implementation project(":badgerlog")
+    annotationProcessor project(":badgerlog")
+}
+
+jmh {
+    resultFormat = 'JSON'
+    resultsFile = file("${buildDir}/reports/jmh/results.json")
+    jvmArgs = [
+            "-Djava.library.path=${projectDir}/build/jni/release", '-Xms6G', '-Xmx6G', "-XX:+FlightRecorder"
+    ] as Iterable<? extends String>
 
-    implementation files("../lib/build/libs/lib.jar")
-    annotationProcessor files("../lib/build/libs/lib.jar")
+    profilers = ['gc']
 }
 
 tasks.register('simulate') {
@@ -103,11 +117,12 @@ tasks.register('buildLib', GradleBuild) {
 }
 
 sourceSets {
-    files("../lib/build/libs/lib-sources.jar")
+    project(":badgerlog")
 }
 
 test{
     useJUnitPlatform()
+    maxHeapSize = "4g"
     systemProperty 'junit.jupiter.extensions.autodetection.enabled', 'true'
 }
 

test-project/settings.gradle

@@ -26,5 +26,8 @@ pluginManagement {
     }
 }
 
+include 'badgerlog'
+project(':badgerlog').projectDir = file('../lib')
+
 Properties props = System.getProperties()
 props.setProperty("org.gradle.internal.native.headers.unresolved.dependencies.ignore", "true")