reduce allocations for group by result tags (#1120)

Avoid copying to new map for the group by.
Netflix · Mar 14, 2024 · b7a5cc5 · b7a5cc5
1 parent 33036d4
commit b7a5cc5
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 10 deletions.
diff --git a/spectator-reg-atlas/src/main/java/com/netflix/spectator/atlas/impl/DataExpr.java b/spectator-reg-atlas/src/main/java/com/netflix/spectator/atlas/impl/DataExpr.java
@@ -480,6 +480,10 @@ final class GroupBy implements DataExpr {
       this.keys = keys;
     }
 
+    AggregateFunction aggregateFunction() {
+      return af;
+    }
+
     Set<String> keys() {
       return keys;
     }

diff --git a/spectator-reg-atlas/src/main/java/com/netflix/spectator/atlas/impl/Evaluator.java b/spectator-reg-atlas/src/main/java/com/netflix/spectator/atlas/impl/Evaluator.java
@@ -23,7 +23,6 @@
 import org.slf4j.LoggerFactory;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -173,23 +172,30 @@ public EvalPayload eval(long timestamp) {
           consolidator.update(timestamp, Double.NaN);
           final double v = consolidator.value(timestamp);
           if (!Double.isNaN(v)) {
-            Map<String, String> tags = Collections.emptyMap();
+            Map<String, String> tags =  null;
             if (expr instanceof DataExpr.GroupBy) {
               // Aggregation functions only use tags based on the expression. Avoid overhead of
               // considering the tags for the data.
-              Set<String> keys = ((DataExpr.GroupBy) expr).keys();
+              DataExpr.GroupBy by = (DataExpr.GroupBy) expr;
+              Set<String> keys = by.keys();
               tags = idMapper.apply(entry.getKey(), keys);
               putCommonTags(tags, keys);
+              if (tags.size() < keys.size()) {
+                // When performing a group by, datapoints missing tag used for the grouping
+                // should be ignored
+                tags = null;
+              } else {
+                tags.putAll(by.aggregateFunction().queryTags());
+              }
+            } else if (expr instanceof DataExpr.AggregateFunction) {
+              DataExpr.AggregateFunction af = (DataExpr.AggregateFunction) expr;
+              tags = new HashMap<>(af.resultTags(af.queryTags()));
             }
             if (delayGaugeAggr && consolidator.isGauge()) {
-              // When performing a group by, datapoints missing tag used for the grouping
-              // should be ignored
-              Map<String, String> rs = expr.resultTags(tags);
-              if (rs != null) {
-                Map<String, String> resultTags = new HashMap<>(rs);
-                resultTags.put("atlas.aggr", idHash(entry.getKey()));
+              if (tags != null) {
+                tags.put("atlas.aggr", idHash(entry.getKey()));
                 double acc = expr.isCount() ? 1.0 : v;
-                metrics.add(new EvalPayload.Metric(subId, resultTags, acc));
+                metrics.add(new EvalPayload.Metric(subId, tags, acc));
               }
             } else {
               TagsValuePair p = new TagsValuePair(tags, v);