Merge pull request #1486 from NASA-AMMOS/naming-efficiency

Naming efficiency

contrib/src/main/java/gov/nasa/jpl/aerie/contrib/streamline/core/CellRefV2.java

@@ -39,8 +39,8 @@ public Cell<D> duplicate(Cell<D> cell) {
       public void apply(Cell<D> cell, E effect) {
         cell.initialDynamics = effect.apply(cell.dynamics).match(
             ErrorCatching::success,
-            error -> failure(new RuntimeException(
-                "Applying '%s' failed.".formatted(getEffectName(effect)), error)));
+                error -> failure(new RuntimeException(
+                    "Applying effect '%s' failed.".formatted(getName(effect, null)), error)));
         cell.dynamics = cell.initialDynamics;
         cell.elapsedTime = ZERO;
       }
@@ -120,7 +120,7 @@ public DynamicsEffect<D> empty() {
       @Override
       public DynamicsEffect<D> sequentially(final DynamicsEffect<D> prefix, final DynamicsEffect<D> suffix) {
         final DynamicsEffect<D> result = x -> suffix.apply(prefix.apply(x));
-        name(result, "(%s) then (%s)".formatted(getEffectName(prefix), getEffectName(suffix)));
+        name(result, "(%s) then (%s)", prefix, suffix);
         return result;
       }
 
@@ -135,22 +135,17 @@ public DynamicsEffect<D> concurrently(final DynamicsEffect<D> left, final Dynami
                   return failure(e);
                 }
               };
-          name(result, "(%s) and (%s)".formatted(getEffectName(left), getEffectName(right)));
+          name(result, "(%s) and (%s)", left, right);
           return result;
         } catch (Throwable e) {
           final DynamicsEffect<D> result = $ -> failure(e);
-          name(result, "Failed to combine concurrent effects: (%s) and (%s)".formatted(
-                  getEffectName(left), getEffectName(right)));
+          name(result, "Failed to combine concurrent effects: (%s) and (%s)", left, right);
           return result;
         }
       }
     };
   }
 
-  private static <D extends Dynamics<?, D>, E extends DynamicsEffect<D>> String getEffectName(E effect) {
-    return getName(effect).orElse("anonymous effect");
-  }
-
   public static class Cell<D> {
     public ErrorCatching<Expiring<D>> initialDynamics;
     public ErrorCatching<Expiring<D>> dynamics;

contrib/src/main/java/gov/nasa/jpl/aerie/contrib/streamline/core/Expiry.java

@@ -20,8 +20,14 @@ public static Expiry expiry(Optional<Duration> value) {
   }
 
   public Expiry or(Expiry other) {
-    return expiry(
-        Stream.concat(value().stream(), other.value().stream()).reduce(Duration::min));
+    // If this has a value...
+    //   If other has a value, compare and return the minimum
+    //   Else other is NEVER, so return this
+    // Else (this is NEVER), so return other
+    return this.value.map(thisValue ->
+            other.value.map(otherValue -> Expiry.at(Duration.min(thisValue, otherValue)))
+                    .orElse(this))
+            .orElse(other);
   }
 
   public Expiry minus(Duration t) {

contrib/src/main/java/gov/nasa/jpl/aerie/contrib/streamline/core/MutableResource.java

@@ -22,8 +22,7 @@
 public interface MutableResource<D extends Dynamics<?, D>> extends Resource<D> {
   void emit(DynamicsEffect<D> effect);
   default void emit(String effectName, DynamicsEffect<D> effect) {
-    name(effect, effectName);
-    emit(effect);
+    emit(name(effect, effectName));
   }
 
   static <D extends Dynamics<?, D>> MutableResource<D> resource(D initial) {
@@ -47,21 +46,18 @@ static <D extends Dynamics<?, D>> MutableResource<D> resource(ErrorCatching<Expi
 
       @Override
       public void emit(final DynamicsEffect<D> effect) {
-        augmentEffectName(effect);
-        cell.emit(effect);
+        // NOTE: The strange pattern of naming effect::apply is to create a new object, identical in behavior to effect,
+        //   which we can assign a more informative name without actually getting the name of effect.
+        // Replacing effect::apply with effect would create a self-loop in the naming graph on effect, which isn't allowed.
+        // Using Naming.getName to get effect's current name and use that when elaborating is correct but potentially slow,
+        //   depending on how deep the naming graph is.
+        cell.emit(name(effect::apply, "%s on %s" + Context.get().stream().map(c -> " during " + c).collect(joining()), effect, this));
       }
 
       @Override
       public ErrorCatching<Expiring<D>> getDynamics() {
         return cell.get().dynamics;
       }
-
-      private void augmentEffectName(DynamicsEffect<D> effect) {
-        String effectName = getName(effect).orElse("anonymous effect");
-        String resourceName = getName(this).orElse("anonymous resource");
-        String augmentedName = effectName + " on " + resourceName + Context.get().stream().map(c -> " during " + c).collect(joining());
-        name(effect, augmentedName);
-      }
     };
     if (MutableResourceFlags.DETECT_BUSY_CELLS) {
       result = profileEffects(result);
@@ -73,11 +69,11 @@ private void augmentEffectName(DynamicsEffect<D> effect) {
   }
 
   static <D extends Dynamics<?, D>> void set(MutableResource<D> resource, D newDynamics) {
-    resource.emit("Set " + newDynamics, DynamicsMonad.effect(x -> newDynamics));
+    resource.emit(name(DynamicsMonad.effect(x -> newDynamics), "Set %s", newDynamics));
   }
 
   static <D extends Dynamics<?, D>> void set(MutableResource<D> resource, Expiring<D> newDynamics) {
-    resource.emit("Set " + newDynamics, ErrorCatchingMonad.<Expiring<D>, Expiring<D>>map($ -> newDynamics)::apply);
+    resource.emit(name(ErrorCatchingMonad.<Expiring<D>, Expiring<D>>map($ -> newDynamics)::apply, "Set %s", newDynamics));
   }
 
   /**

contrib/src/main/java/gov/nasa/jpl/aerie/contrib/streamline/core/Resources.java

@@ -183,8 +183,7 @@ public static Condition expires(Resource<?> resource) {
    */
   public static <D extends Dynamics<?, D>> void forward(Resource<D> source, MutableResource<D> destination) {
     wheneverDynamicsChange(source, s -> destination.emit(
-            "Forward %s dynamics: %s".formatted(getName(source).orElse("anonymous"), s),
-            $ -> s));
+            name($ -> s, "Forward %s dynamics to %s", source, destination)));
     addDependency(destination, source);
   }
 

contrib/src/main/java/gov/nasa/jpl/aerie/contrib/streamline/debugging/Context.java

@@ -2,8 +2,7 @@
 
 import gov.nasa.jpl.aerie.merlin.protocol.types.Unit;
 
-import java.util.ArrayDeque;
-import java.util.Deque;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.function.Supplier;
 
@@ -15,7 +14,7 @@
 public final class Context {
   private Context() {}
 
-  private static final ThreadLocal<Deque<String>> contexts = ThreadLocal.withInitial(ArrayDeque::new);
+  private static final ThreadLocal<List<String>> contexts = ThreadLocal.withInitial(List::of);
 
   /**
    * @see Context#inContext(String, Supplier)
@@ -30,13 +29,19 @@ public static void inContext(String contextName, Runnable action) {
    */
   public static <R> R inContext(String contextName, Supplier<R> action) {
     // Using a thread-local context stack maintains isolation for threaded tasks.
+    var outerContext = contexts.get();
     try {
-      contexts.get().push(contextName);
+      // Since copying the context into spawned children actually dominates performance,
+      // use an effectively-immutable list for the stack, and copy it to push/pop a layer.
+      var innerContext = new ArrayList<String>(outerContext.size() + 1);
+      innerContext.addAll(outerContext);
+      innerContext.add(contextName);
+      contexts.set(innerContext);
       return action.get();
       // TODO: Should we add a catch clause here that would add context to the error?
     } finally {
       // Doing the tear-down in a finally block maintains isolation for replaying tasks.
-      contexts.get().pop();
+      contexts.set(outerContext);
     }
   }
 
@@ -61,12 +66,12 @@ public static void inContext(List<String> contextStack, Runnable action) {
    * @see Context#contextualized
    */
   public static <R> R inContext(List<String> contextStack, Supplier<R> action) {
-    if (contextStack.isEmpty()) {
+    var outerContext = contexts.get();
+    try {
+      contexts.set(contextStack);
       return action.get();
-    } else {
-      int n = contextStack.size() - 1;
-      return inContext(contextStack.get(n), () ->
-          inContext(contextStack.subList(0, n), action));
+    } finally {
+      contexts.set(outerContext);
     }
   }
 
@@ -133,9 +138,15 @@ public static <R> Supplier<R> contextualized(String childContext, Supplier<R> ac
 
   /**
    * Returns the list of contexts, from innermost context out.
+   *
+   * <p>
+   *     Note: For efficiency, this returns the actual context stack, not a copy.
+   *     Do not modify the result of this method!
+   *     Doing so may corrupt the context-tracking system.
+   * </p>
    */
   public static List<String> get() {
-    return contexts.get().stream().toList();
+    return contexts.get();
   }
 
   private static Supplier<Unit> asSupplier(Runnable action) {

contrib/src/main/java/gov/nasa/jpl/aerie/contrib/streamline/debugging/Naming.java

@@ -28,15 +28,19 @@ private Naming() {}
   // Use a WeakHashMap so that naming a thing doesn't prevent it from being garbage-collected.
   private static final WeakHashMap<Object, Function<NamingContext, Optional<String>>> NAMES = new WeakHashMap<>();
 
-  private record NamingContext(Set<Object> visited, Optional<String> anonymousName) {
+  private record NamingContext(Set<Object> visited, Function<Object, Optional<String>> anonymousName) {
     NamingContext visit(Object thing) {
       var newVisited = new HashSet<>(visited);
       newVisited.add(thing);
       return new NamingContext(newVisited, anonymousName);
     }
 
+    public NamingContext() {
+      this(Set.of(), $ -> Optional.empty());
+    }
+
     public NamingContext(String anonymousName) {
-      this(Set.of(), Optional.ofNullable(anonymousName));
+      this(Set.of(), anonymousName == null ? $ -> Optional.of(Objects.toString($)) : $ -> Optional.of(anonymousName));
     }
   }
 
@@ -52,7 +56,9 @@ public static <T> T name(T thing, String nameFormat, Object... args) {
       for (int i = 0; i < args$.length; ++i) {
         var argName$ = Optional.ofNullable(args$[i].get())
                 .flatMap(argRef -> getName(argRef, context));
-        if (argName$.isEmpty()) return context.anonymousName();
+        if (argName$.isEmpty()) {
+          return context.anonymousName.apply(args$[i].get());
+        }
         argNames[i] = argName$.get();
       }
       return Optional.of(nameFormat.formatted(argNames));
@@ -66,9 +72,14 @@ public static <T> T name(T thing, String nameFormat, Object... args) {
    * returns empty.
    */
   public static Optional<String> getName(Object thing) {
-    return getName(thing, new NamingContext(null));
+    return getName(thing, new NamingContext());
   }
 
+  // TODO - build a version of getName, or make this the default behavior, (maybe if anonymousName is null?)
+  //   which calls Object.toString on things that don't otherwise have a name.
+  //   This supports the use of literals and dynamics objects in effect names,
+  //   without needing to eagerly compute their names, which involves costly string ops.
+
   /**
    * Get the name for thing.
    * Use anonymousName for anything without a name instead of returning empty.
@@ -80,8 +91,8 @@ public static String getName(Object thing, String anonymousName) {
 
   private static Optional<String> getName(Object thing, NamingContext context) {
     return context.visited.contains(thing)
-            ? context.anonymousName
-            : NAMES.getOrDefault(thing, NamingContext::anonymousName).apply(context.visit(thing));
+            ? context.anonymousName.apply(thing)
+            : NAMES.getOrDefault(thing, ctx -> ctx.anonymousName.apply(thing)).apply(context.visit(thing));
   }
 
   public static String argsFormat(Collection<?> collection) {

contrib/src/main/java/gov/nasa/jpl/aerie/contrib/streamline/debugging/Profiling.java

@@ -152,7 +152,7 @@ public static <D extends Dynamics<?, D>> MutableResource<D> profileEffects(Mutab
     MutableResource<D> result = new MutableResource<>() {
       @Override
       public void emit(DynamicsEffect<D> effect) {
-        resource.emit(x -> accrue(effectsEmitted, getName(this, "..."), () -> effect.apply(x)));
+        resource.emit(x -> accrue(effectsEmitted, getName(this, null), () -> effect.apply(x)));
       }
 
       @Override
@@ -167,7 +167,7 @@ public ErrorCatching<Expiring<D>> getDynamics() {
   private static Supplier<String> computeName(String explicitName, Object profiledThing) {
     return explicitName != null
             ? () -> explicitName
-            : () -> getName(profiledThing, "...");
+            : () -> getName(profiledThing, null);
   }
 
   private static long ANONYMOUS_ID = 0;

contrib/src/main/java/gov/nasa/jpl/aerie/contrib/streamline/modeling/black_box/Approximation.java

@@ -46,7 +46,7 @@ public static <D extends Dynamics<?, D>, E extends Dynamics<?, E>> Resource<E> a
   private static <D extends Dynamics<?, D>, E extends Dynamics<?, E>> void updateApproximation(
       ErrorCatching<Expiring<D>> resourceDynamics, Function<Expiring<D>, Expiring<E>> approximation, MutableResource<E> result) {
     var newDynamics = resourceDynamics.map(approximation);
-    result.emit("Update approximation to " + newDynamics, $ -> newDynamics);
+    result.emit(name($ -> newDynamics, "Update approximation to %s", newDynamics));
   }
 
   /**

contrib/src/main/java/gov/nasa/jpl/aerie/contrib/streamline/modeling/black_box/UnstructuredResources.java

@@ -23,7 +23,8 @@ private UnstructuredResources() {}
 
   public static <A> Resource<Unstructured<A>> constant(A value) {
     var result = UnstructuredResourceApplicative.pure(value);
-    name(result, value.toString());
+    // Use an edge in the naming graph directly to the value to lazily compute the name
+    name(result, "%s", value);
     return result;
   }
 

contrib/src/main/java/gov/nasa/jpl/aerie/contrib/streamline/modeling/discrete/DiscreteEffects.java

@@ -8,6 +8,7 @@
 import java.util.Optional;
 
 import static gov.nasa.jpl.aerie.contrib.streamline.core.Resources.currentValue;
+import static gov.nasa.jpl.aerie.contrib.streamline.debugging.Naming.name;
 import static gov.nasa.jpl.aerie.contrib.streamline.modeling.discrete.monads.DiscreteDynamicsMonad.effect;
 
 public final class DiscreteEffects {
@@ -19,7 +20,7 @@ private DiscreteEffects() {}
    * Set the resource to the given value.
    */
   public static <A> void set(MutableResource<Discrete<A>> resource, A newValue) {
-    resource.emit("Set " + newValue, effect(x -> newValue));
+    resource.emit(name(effect(x -> newValue), "Set %s", newValue));
   }
 
   // Flag/Switch style operations
@@ -58,7 +59,7 @@ public static void increment(MutableResource<Discrete<Integer>> resource) {
    * Add the given amount to the resource's value.
    */
   public static void increment(MutableResource<Discrete<Integer>> resource, int amount) {
-    resource.emit("Increment by " + amount, effect(x -> x + amount));
+    resource.emit(name(effect(x -> x + amount), "Increment by %s", amount));
   }
 
   /**
@@ -72,7 +73,7 @@ public static void decrement(MutableResource<Discrete<Integer>> resource) {
    * Subtract the given amount from the resource's value.
    */
   public static void decrement(MutableResource<Discrete<Integer>> resource, int amount) {
-    resource.emit("Decrement by " + amount, effect(x -> x - amount));
+    resource.emit(name(effect(x -> x - amount), "Decrement by %s", amount));
   }
 
   // General numeric resources
@@ -81,14 +82,14 @@ public static void decrement(MutableResource<Discrete<Integer>> resource, int am
    * Add amount to resource's value
    */
   public static void increase(MutableResource<Discrete<Double>> resource, double amount) {
-    resource.emit("Increase by " + amount, effect(x -> x + amount));
+    resource.emit(name(effect(x -> x + amount), "Increase by %s", amount));
   }
 
   /**
    * Subtract amount from resource's value
    */
   public static void decrease(MutableResource<Discrete<Double>> resource, double amount) {
-    resource.emit("Decrease by " + amount, effect(x -> x - amount));
+    resource.emit(name(effect(x -> x - amount), "Decrease by %s", amount));
   }
 
   // Queue style operations, mirroring the Queue interface
@@ -97,11 +98,11 @@ public static void decrease(MutableResource<Discrete<Double>> resource, double a
    * Add element to the end of the queue resource
    */
   public static <T> void add(MutableResource<Discrete<List<T>>> resource, T element) {
-    resource.emit("Add %s to queue".formatted(element), effect(q -> {
+    resource.emit(name(effect(q -> {
       var q$ = new LinkedList<>(q);
       q$.add(element);
       return q$;
-    }));
+    }), "Add %s to queue", element));
   }
 
   /**
@@ -115,14 +116,14 @@ public static <T> Optional<T> remove(MutableResource<Discrete<List<T>>> resource
     if (currentQueue.isEmpty()) return Optional.empty();
 
     final T result = currentQueue.get(currentQueue.size() - 1);
-    resource.emit("Remove %s from queue".formatted(result), effect(q -> {
+    resource.emit(name(effect(q -> {
       var q$ = new LinkedList<>(q);
       T purportedResult = q$.removeLast();
       if (!result.equals(purportedResult)) {
         throw new IllegalStateException("Detected effect conflicting with queue remove operation");
       }
       return q$;
-    }));
+    }), "Remove %s from queue", result));
     return Optional.of(result);
   }
 
@@ -132,14 +133,14 @@ public static <T> Optional<T> remove(MutableResource<Discrete<List<T>>> resource
    * Subtract the given amount from resource.
    */
   public static void consume(MutableResource<Discrete<Double>> resource, double amount) {
-    resource.emit("Consume " + amount, effect(x -> x - amount));
+    resource.emit(name(effect(x -> x - amount), "Consume %s", amount));
   }
 
   /**
    * Add the given amount to resource.
    */
   public static void restore(MutableResource<Discrete<Double>> resource, double amount) {
-    resource.emit("Restore " + amount, effect(x -> x + amount));
+    resource.emit(name(effect(x -> x + amount), "Restore %s", amount));
   }
 
   // Non-consumable style operations