From e85431043c8d4215bde34714b9e0ea5913302129 Mon Sep 17 00:00:00 2001 From: jean-philippe bempel Date: Wed, 28 May 2025 11:20:40 +0200 Subject: [PATCH] Optimized allocations for collection filter functions We were allocating some unnecessary objects for each item in the collection we attempt to filter with any, all and filter expression. introduced withExtension/removeExtension method to reduce those allocations --- .../bootstrap/debugger/CapturedContext.java | 10 +-- .../debugger/el/ValueReferenceResolver.java | 4 ++ .../FilterCollectionExpression.java | 52 +++++++++------- .../el/expressions/HasAllExpression.java | 60 ++++++++++-------- .../el/expressions/HasAnyExpression.java | 61 ++++++++++--------- 5 files changed, 107 insertions(+), 80 deletions(-) diff --git a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/CapturedContext.java b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/CapturedContext.java index fe7b336f6ff..406dae182fb 100644 --- a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/CapturedContext.java +++ b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/CapturedContext.java @@ -182,12 +182,14 @@ public ValueReferenceResolver withExtensions(Map extensions) { return new CapturedContext(this, extensions); } - public void removeExtension(String name) { - extensions.remove(name); + @Override + public void addExtension(String name, Object value) { + extensions.put(name, value); } - private void addExtension(String name, Object value) { - extensions.put(name, value); + @Override + public void removeExtension(String name) { + extensions.remove(name); } public void addArguments(CapturedValue[] values) { diff --git a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/el/ValueReferenceResolver.java b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/el/ValueReferenceResolver.java index af355600080..8dae8541db7 100644 --- a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/el/ValueReferenceResolver.java +++ b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/el/ValueReferenceResolver.java @@ -8,6 +8,10 @@ public interface ValueReferenceResolver { Object getMember(Object target, String name); + default void addExtension(String name, Object value) {} + + default void removeExtension(String name) {} + default ValueReferenceResolver withExtensions(Map extensions) { return this; } diff --git a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/FilterCollectionExpression.java b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/FilterCollectionExpression.java index d9534f068b5..d532587c744 100644 --- a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/FilterCollectionExpression.java +++ b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/FilterCollectionExpression.java @@ -17,7 +17,6 @@ import datadog.trace.bootstrap.debugger.el.ValueReferences; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -48,41 +47,52 @@ public CollectionValue evaluate(ValueReferenceResolver valueRefResolver) { checkSupportedList(materialized, this); Collection filtered = new ArrayList<>(); int len = materialized.count(); - for (int i = 0; i < len; i++) { - Object value = materialized.get(i).getValue(); - if (filterExpression.evaluate( - valueRefResolver.withExtensions( - Collections.singletonMap(ValueReferences.ITERATOR_EXTENSION_NAME, value)))) { - filtered.add(value); + try { + for (int i = 0; i < len; i++) { + Object value = materialized.get(i).getValue(); + valueRefResolver.addExtension(ValueReferences.ITERATOR_EXTENSION_NAME, value); + if (filterExpression.evaluate(valueRefResolver)) { + filtered.add(value); + } } + } finally { + valueRefResolver.removeExtension(ValueReferences.ITERATOR_EXTENSION_NAME); } return new ListValue(filtered); } else if (collectionValue instanceof MapValue) { MapValue materialized = (MapValue) collectionValue; checkSupportedMap(materialized, this); Map filtered = new HashMap<>(); - for (Value key : materialized.getKeys()) { - Value value = key.isUndefined() ? Value.undefinedValue() : materialized.get(key); - Map valueRefExtensions = new HashMap<>(); - valueRefExtensions.put(ValueReferences.KEY_EXTENSION_NAME, key); - valueRefExtensions.put(ValueReferences.VALUE_EXTENSION_NAME, value); - valueRefExtensions.put( - ValueReferences.ITERATOR_EXTENSION_NAME, new MapValue.Entry(key, value)); - if (filterExpression.evaluate(valueRefResolver.withExtensions(valueRefExtensions))) { - filtered.put(key.getValue(), value.getValue()); + try { + for (Value key : materialized.getKeys()) { + Value value = key.isUndefined() ? Value.undefinedValue() : materialized.get(key); + valueRefResolver.addExtension(ValueReferences.KEY_EXTENSION_NAME, key); + valueRefResolver.addExtension(ValueReferences.VALUE_EXTENSION_NAME, value); + valueRefResolver.addExtension( + ValueReferences.ITERATOR_EXTENSION_NAME, new MapValue.Entry(key, value)); + if (filterExpression.evaluate(valueRefResolver)) { + filtered.put(key.getValue(), value.getValue()); + } } + } finally { + valueRefResolver.removeExtension(ValueReferences.KEY_EXTENSION_NAME); + valueRefResolver.removeExtension(ValueReferences.VALUE_EXTENSION_NAME); + valueRefResolver.removeExtension(ValueReferences.ITERATOR_EXTENSION_NAME); } return new MapValue(filtered); } else if (collectionValue instanceof SetValue) { SetValue materialized = (SetValue) collectionValue; Collection filtered = new HashSet<>(); Set setHolder = checkSupportedSet(materialized, this); - for (Object value : setHolder) { - if (filterExpression.evaluate( - valueRefResolver.withExtensions( - Collections.singletonMap(ValueReferences.ITERATOR_EXTENSION_NAME, value)))) { - filtered.add(value); + try { + for (Object value : setHolder) { + valueRefResolver.addExtension(ValueReferences.ITERATOR_EXTENSION_NAME, value); + if (filterExpression.evaluate(valueRefResolver)) { + filtered.add(value); + } } + } finally { + valueRefResolver.removeExtension(ValueReferences.ITERATOR_EXTENSION_NAME); } return new SetValue(filtered); } diff --git a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/HasAllExpression.java b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/HasAllExpression.java index e22a63ff2ab..ba3c859d95a 100644 --- a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/HasAllExpression.java +++ b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/HasAllExpression.java @@ -14,9 +14,6 @@ import com.datadog.debugger.el.values.SetValue; import datadog.trace.bootstrap.debugger.el.ValueReferenceResolver; import datadog.trace.bootstrap.debugger.el.ValueReferences; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; import java.util.Set; /** @@ -42,15 +39,17 @@ public Boolean evaluate(ValueReferenceResolver valueRefResolver) { return Boolean.TRUE; } int len = collection.count(); - for (int i = 0; i < len; i++) { - Value val = collection.get(i); - if (!filterPredicateExpression.evaluate( - valueRefResolver.withExtensions( - Collections.singletonMap(ValueReferences.ITERATOR_EXTENSION_NAME, val)))) { - return Boolean.FALSE; + try { + for (int i = 0; i < len; i++) { + valueRefResolver.addExtension(ValueReferences.ITERATOR_EXTENSION_NAME, collection.get(i)); + if (!filterPredicateExpression.evaluate(valueRefResolver)) { + return Boolean.FALSE; + } } + return Boolean.TRUE; + } finally { + valueRefResolver.removeExtension(ValueReferences.ITERATOR_EXTENSION_NAME); } - return Boolean.TRUE; } if (value instanceof MapValue) { MapValue map = (MapValue) value; @@ -59,19 +58,23 @@ public Boolean evaluate(ValueReferenceResolver valueRefResolver) { // always return TRUE for empty values (cf vacuous truth, see also Stream::allMatch) return Boolean.TRUE; } - for (Value key : map.getKeys()) { - Value val = key.isUndefined() ? Value.undefinedValue() : map.get(key); - Map valueRefExtensions = new HashMap<>(); - valueRefExtensions.put(ValueReferences.KEY_EXTENSION_NAME, key); - valueRefExtensions.put(ValueReferences.VALUE_EXTENSION_NAME, val); - valueRefExtensions.put( - ValueReferences.ITERATOR_EXTENSION_NAME, new MapValue.Entry(key, val)); - if (!filterPredicateExpression.evaluate( - valueRefResolver.withExtensions(valueRefExtensions))) { - return Boolean.FALSE; + try { + for (Value key : map.getKeys()) { + Value val = key.isUndefined() ? Value.undefinedValue() : map.get(key); + valueRefResolver.addExtension(ValueReferences.KEY_EXTENSION_NAME, key); + valueRefResolver.addExtension(ValueReferences.VALUE_EXTENSION_NAME, val); + valueRefResolver.addExtension( + ValueReferences.ITERATOR_EXTENSION_NAME, new MapValue.Entry(key, val)); + if (!filterPredicateExpression.evaluate(valueRefResolver)) { + return Boolean.FALSE; + } } + return Boolean.TRUE; + } finally { + valueRefResolver.removeExtension(ValueReferences.ITERATOR_EXTENSION_NAME); + valueRefResolver.removeExtension(ValueReferences.KEY_EXTENSION_NAME); + valueRefResolver.removeExtension(ValueReferences.VALUE_EXTENSION_NAME); } - return Boolean.TRUE; } if (value instanceof SetValue) { SetValue set = (SetValue) value; @@ -80,14 +83,17 @@ public Boolean evaluate(ValueReferenceResolver valueRefResolver) { // always return TRUE for empty values (cf vacuous truth, see also Stream::allMatch) return Boolean.TRUE; } - for (Object val : setHolder) { - if (!filterPredicateExpression.evaluate( - valueRefResolver.withExtensions( - Collections.singletonMap(ValueReferences.ITERATOR_EXTENSION_NAME, val)))) { - return Boolean.FALSE; + try { + for (Object val : setHolder) { + valueRefResolver.addExtension(ValueReferences.ITERATOR_EXTENSION_NAME, val); + if (!filterPredicateExpression.evaluate(valueRefResolver)) { + return Boolean.FALSE; + } } + return Boolean.TRUE; + } finally { + valueRefResolver.removeExtension(ValueReferences.ITERATOR_EXTENSION_NAME); } - return Boolean.TRUE; } throw new EvaluationException( "Unsupported collection class: " + value.getValue().getClass().getTypeName(), print(this)); diff --git a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/HasAnyExpression.java b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/HasAnyExpression.java index 921c7d4cab2..d2ea133ebe0 100644 --- a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/HasAnyExpression.java +++ b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/HasAnyExpression.java @@ -14,9 +14,6 @@ import com.datadog.debugger.el.values.SetValue; import datadog.trace.bootstrap.debugger.el.ValueReferenceResolver; import datadog.trace.bootstrap.debugger.el.ValueReferences; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; import java.util.Set; /** @@ -43,15 +40,17 @@ public Boolean evaluate(ValueReferenceResolver valueRefResolver) { return Boolean.FALSE; } int len = collection.count(); - for (int i = 0; i < len; i++) { - Value val = collection.get(i); - if (filterPredicateExpression.evaluate( - valueRefResolver.withExtensions( - Collections.singletonMap(ValueReferences.ITERATOR_EXTENSION_NAME, val)))) { - return Boolean.TRUE; + try { + for (int i = 0; i < len; i++) { + valueRefResolver.addExtension(ValueReferences.ITERATOR_EXTENSION_NAME, collection.get(i)); + if (filterPredicateExpression.evaluate(valueRefResolver)) { + return Boolean.TRUE; + } } + return Boolean.FALSE; + } finally { + valueRefResolver.removeExtension(ValueReferences.ITERATOR_EXTENSION_NAME); } - return Boolean.FALSE; } if (value instanceof MapValue) { MapValue map = (MapValue) value; @@ -59,19 +58,23 @@ public Boolean evaluate(ValueReferenceResolver valueRefResolver) { if (map.isEmpty()) { return Boolean.FALSE; } - for (Value key : map.getKeys()) { - Value val = key.isUndefined() ? Value.undefinedValue() : map.get(key); - Map valueRefExtensions = new HashMap<>(); - valueRefExtensions.put(ValueReferences.KEY_EXTENSION_NAME, key); - valueRefExtensions.put(ValueReferences.VALUE_EXTENSION_NAME, val); - valueRefExtensions.put( - ValueReferences.ITERATOR_EXTENSION_NAME, new MapValue.Entry(key, val)); - if (filterPredicateExpression.evaluate( - valueRefResolver.withExtensions(valueRefExtensions))) { - return Boolean.TRUE; + try { + for (Value key : map.getKeys()) { + Value val = key.isUndefined() ? Value.undefinedValue() : map.get(key); + valueRefResolver.addExtension(ValueReferences.KEY_EXTENSION_NAME, key); + valueRefResolver.addExtension(ValueReferences.VALUE_EXTENSION_NAME, val); + valueRefResolver.addExtension( + ValueReferences.ITERATOR_EXTENSION_NAME, new MapValue.Entry(key, val)); + if (filterPredicateExpression.evaluate(valueRefResolver)) { + return Boolean.TRUE; + } } + return Boolean.FALSE; + } finally { + valueRefResolver.removeExtension(ValueReferences.ITERATOR_EXTENSION_NAME); + valueRefResolver.removeExtension(ValueReferences.KEY_EXTENSION_NAME); + valueRefResolver.removeExtension(ValueReferences.VALUE_EXTENSION_NAME); } - return Boolean.FALSE; } if (value instanceof SetValue) { SetValue set = (SetValue) value; @@ -79,15 +82,17 @@ public Boolean evaluate(ValueReferenceResolver valueRefResolver) { if (set.isEmpty()) { return Boolean.FALSE; } - for (Object val : setHolder) { - if (filterPredicateExpression.evaluate( - valueRefResolver.withExtensions( - Collections.singletonMap( - ValueReferences.ITERATOR_EXTENSION_NAME, Value.of(val))))) { - return Boolean.TRUE; + try { + for (Object val : setHolder) { + valueRefResolver.addExtension(ValueReferences.ITERATOR_EXTENSION_NAME, Value.of(val)); + if (filterPredicateExpression.evaluate(valueRefResolver)) { + return Boolean.TRUE; + } } + return Boolean.FALSE; + } finally { + valueRefResolver.removeExtension(ValueReferences.ITERATOR_EXTENSION_NAME); } - return Boolean.FALSE; } throw new EvaluationException( "Unsupported collection class: " + value.getValue().getClass().getTypeName(), print(this));