From 8c121f7aaa0c0aae10f4be6a155479dda4c0e1f2 Mon Sep 17 00:00:00 2001 From: Tyler Gregg Date: Thu, 16 Jan 2025 13:46:01 -0800 Subject: [PATCH] Reuses the EExpressionArgsReader's list of expressions instead of allocating a new one for each invocation. --- .../impl/IonReaderContinuableCoreBinary.java | 33 +++++++------- .../amazon/ion/impl/IonReaderTextSystemX.java | 4 +- .../ion/impl/macro/EExpressionArgsReader.java | 43 ++++++++----------- 3 files changed, 34 insertions(+), 46 deletions(-) diff --git a/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java b/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java index b682be5df..69237ce5a 100644 --- a/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java +++ b/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java @@ -1602,9 +1602,8 @@ private class BinaryEExpressionArgsReader extends EExpressionArgsReader { /** * Reads a single (non-grouped) expression. * @param parameter the parameter. - * @param expressions receives the expressions as they are materialized. */ - private void readSingleExpression(Macro.Parameter parameter, List expressions) { + private void readSingleExpression(Macro.Parameter parameter) { Macro.ParameterEncoding encoding = parameter.getType(); if (encoding == Macro.ParameterEncoding.Tagged) { IonReaderContinuableCoreBinary.super.nextValue(); @@ -1614,15 +1613,14 @@ private void readSingleExpression(Macro.Parameter parameter, List expressions, boolean requireSingleton) { + private void readGroupExpression(Macro.Parameter parameter, boolean requireSingleton) { Macro.ParameterEncoding encoding = parameter.getType(); if (encoding == Macro.ParameterEncoding.Tagged) { enterTaggedArgumentGroup(); @@ -1636,7 +1634,7 @@ private void readGroupExpression(Macro.Parameter parameter, List expressions) { + private void addVoidExpression() { int startIndex = expressions.size(); expressions.add(new Expression.ExpressionGroup(startIndex, startIndex + 1)); } @Override - protected void readParameter(Macro.Parameter parameter, long parameterPresence, List expressions, boolean isTrailing) { + protected void readParameter(Macro.Parameter parameter, long parameterPresence, boolean isTrailing) { switch (parameter.getCardinality()) { case ZeroOrOne: if (parameterPresence == PresenceBitmap.EXPRESSION) { - readSingleExpression(parameter, expressions); + readSingleExpression(parameter); } else if (parameterPresence == PresenceBitmap.VOID) { - addVoidExpression(expressions); + addVoidExpression(); } else if (parameterPresence == PresenceBitmap.GROUP) { - readGroupExpression(parameter, expressions, true); + readGroupExpression(parameter, true); } else { throw new IllegalStateException("Unreachable: presence bitmap validated but reserved bits found."); } break; case ExactlyOne: // TODO determine if a group with a single element is valid here. - readSingleExpression(parameter, expressions); + readSingleExpression(parameter); break; case OneOrMore: if (parameterPresence == PresenceBitmap.EXPRESSION) { - readSingleExpression(parameter, expressions); + readSingleExpression(parameter); } else if (parameterPresence == PresenceBitmap.GROUP) { - readGroupExpression(parameter, expressions, false); + readGroupExpression(parameter, false); } else { throw new IonException(String.format( "Invalid void argument for non-voidable parameter: %s", @@ -1693,11 +1690,11 @@ protected void readParameter(Macro.Parameter parameter, long parameterPresence, break; case ZeroOrMore: if (parameterPresence == PresenceBitmap.EXPRESSION) { - readSingleExpression(parameter, expressions); + readSingleExpression(parameter); } else if (parameterPresence == PresenceBitmap.GROUP) { - readGroupExpression(parameter, expressions, false); + readGroupExpression(parameter, false); } else if (parameterPresence == PresenceBitmap.VOID) { - addVoidExpression(expressions); + addVoidExpression(); } else { throw new IllegalStateException("Unreachable: presence bitmap validated but reserved bits found."); } diff --git a/src/main/java/com/amazon/ion/impl/IonReaderTextSystemX.java b/src/main/java/com/amazon/ion/impl/IonReaderTextSystemX.java index e636c0a0d..f4144f5ce 100644 --- a/src/main/java/com/amazon/ion/impl/IonReaderTextSystemX.java +++ b/src/main/java/com/amazon/ion/impl/IonReaderTextSystemX.java @@ -1251,14 +1251,14 @@ private class TextEExpressionArgsReader extends EExpressionArgsReader { } @Override - protected void readParameter(Macro.Parameter parameter, long parameterPresence, List expressions, boolean isTrailing) { + protected void readParameter(Macro.Parameter parameter, long parameterPresence, boolean isTrailing) { if (IonReaderTextSystemX.this.nextRaw() == null) { // Add an empty expression group if nothing present. int index = expressions.size() + 1; expressions.add(new Expression.ExpressionGroup(index, index)); return; } - readValueAsExpression(isTrailing && parameter.getCardinality().canBeMulti, expressions); + readValueAsExpression(isTrailing && parameter.getCardinality().canBeMulti); } @Override diff --git a/src/main/java/com/amazon/ion/impl/macro/EExpressionArgsReader.java b/src/main/java/com/amazon/ion/impl/macro/EExpressionArgsReader.java index 6e2882ee0..15bb6da65 100644 --- a/src/main/java/com/amazon/ion/impl/macro/EExpressionArgsReader.java +++ b/src/main/java/com/amazon/ion/impl/macro/EExpressionArgsReader.java @@ -25,6 +25,9 @@ public abstract class EExpressionArgsReader { private final ReaderAdapter reader; + // Reusable sink for expressions. + protected final List expressions = new ArrayList<>(16); + /** * Constructor. * @param reader the {@link ReaderAdapter} from which to read {@link Expression}s. @@ -82,10 +85,9 @@ public EExpressionArgsReader(ReaderAdapter reader) { * Reads a single parameter to a macro invocation. * @param parameter information about the parameter from the macro signature. * @param parameterPresence the presence bits dedicated to this parameter (unused in text). - * @param expressions receives the expressions as they are materialized. * @param isTrailing true if this parameter is the last one in the signature; otherwise, false (unused in binary). */ - protected abstract void readParameter(Macro.Parameter parameter, long parameterPresence, List expressions, boolean isTrailing); + protected abstract void readParameter(Macro.Parameter parameter, long parameterPresence, boolean isTrailing); /** * Reads the macro's address and attempts to resolve that address to a Macro from the macro table. @@ -104,12 +106,10 @@ public EExpressionArgsReader(ReaderAdapter reader) { * Reads a scalar value from the stream into an expression. * @param type the type of scalar. * @param annotations any annotations on the scalar. - * @param expressions receives the expressions as they are materialized. */ private void readScalarValueAsExpression( IonType type, - List annotations, - List expressions + List annotations ) { Expression.EExpressionBodyExpression expression; if (reader.isNullValue()) { @@ -165,12 +165,10 @@ private void readScalarValueAsExpression( * the MacroEvaluator responsible for evaluating the e-expression to which this container belongs. * @param type the type of container. * @param annotations any annotations on the container. - * @param expressions receives the expressions as they are materialized. */ private void readContainerValueAsExpression( IonType type, - List annotations, - List expressions + List annotations ) { int startIndex = expressions.size(); expressions.add(Expression.Placeholder.INSTANCE); @@ -181,7 +179,7 @@ private void readContainerValueAsExpression( if (type == IonType.STRUCT) { expressions.add(new Expression.FieldName(reader.getFieldNameSymbol())); } - readValueAsExpression(false, expressions); // TODO avoid recursion + readValueAsExpression(false); // TODO avoid recursion } stepOutRaw(); // Overwrite the placeholder with an expression representing the actual type of the container and the @@ -210,15 +208,12 @@ private void readContainerValueAsExpression( /** * Reads the rest of the stream into a single expression group. - * @param expressions receives the expressions as they are materialized. */ - private void readStreamAsExpressionGroup( - List expressions - ) { + private void readStreamAsExpressionGroup() { int startIndex = expressions.size(); expressions.add(Expression.Placeholder.INSTANCE); do { - readValueAsExpression(false, expressions); // TODO avoid recursion + readValueAsExpression(false); // TODO avoid recursion } while (nextRaw()); expressions.set(startIndex, new Expression.ExpressionGroup(startIndex, expressions.size())); } @@ -228,30 +223,28 @@ private void readStreamAsExpressionGroup( * responsible for evaluating the e-expression to which this value belongs. * @param isImplicitRest true if this is the final parameter in the signature, it is variadic, and the format * supports implicit rest parameters (text only); otherwise, false. - * @param expressions receives the expressions as they are materialized. */ - protected void readValueAsExpression(boolean isImplicitRest, List expressions) { + protected void readValueAsExpression(boolean isImplicitRest) { if (isImplicitRest && !isContainerAnExpressionGroup()) { - readStreamAsExpressionGroup(expressions); + readStreamAsExpressionGroup(); return; } else if (isMacroInvocation()) { - collectEExpressionArgs(expressions); // TODO avoid recursion + collectEExpressionArgs(); // TODO avoid recursion return; } IonType type = reader.encodingType(); List annotations = getAnnotations(); if (IonType.isContainer(type) && !reader.isNullValue()) { - readContainerValueAsExpression(type, annotations, expressions); + readContainerValueAsExpression(type, annotations); } else { - readScalarValueAsExpression(type, annotations, expressions); + readScalarValueAsExpression(type, annotations); } } /** * Collects the expressions that compose the current macro invocation. - * @param expressions receives the expressions as they are materialized. */ - private void collectEExpressionArgs(List expressions) { + private void collectEExpressionArgs() { Macro macro = loadMacro(); stepIntoEExpression(); List signature = macro.getSignature(); @@ -263,7 +256,6 @@ private void collectEExpressionArgs(List e readParameter( signature.get(i), presenceBitmap == null ? PresenceBitmap.EXPRESSION : presenceBitmap.get(i), - expressions, i == (numberOfParameters - 1) ); } @@ -276,13 +268,12 @@ private void collectEExpressionArgs(List e * them to the macro evaluator. */ public void beginEvaluatingMacroInvocation(MacroEvaluator macroEvaluator) { - // TODO performance: use a pool of expression lists to avoid repetitive allocations. - List expressions = new ArrayList<>(); + expressions.clear(); // TODO performance: avoid fully materializing all expressions up-front. if (reader.isInStruct()) { expressions.add(new Expression.FieldName(reader.getFieldNameSymbol())); } - collectEExpressionArgs(expressions); + collectEExpressionArgs(); macroEvaluator.initExpansion(expressions); } }