Skip to content

Commit

Permalink
Reuses the EExpressionArgsReader's list of expressions instead of all…
Browse files Browse the repository at this point in the history 
…ocating a new one for each invocation.
  • Loading branch information
@tgregg
tgregg committed Jan 16, 2025
1 parent 3242f09 commit 8c121f7
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 46 deletions.
33 changes: 15 additions & 18 deletions src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Expression.EExpressionBodyExpression> expressions) {
private void readSingleExpression(Macro.Parameter parameter) {
Macro.ParameterEncoding encoding = parameter.getType();
if (encoding == Macro.ParameterEncoding.Tagged) {
IonReaderContinuableCoreBinary.super.nextValue();
Expand All @@ -1614,15 +1613,14 @@ private void readSingleExpression(Macro.Parameter parameter, List<Expression.EEx
if (event == Event.NEEDS_DATA) {
throw new UnsupportedOperationException("TODO: support continuable parsing of macro arguments.");
}
readValueAsExpression(false, expressions);
readValueAsExpression(false);
}

/**
* Reads a group expression.
* @param parameter the parameter.
* @param expressions receives the expressions as they are materialized.
*/
private void readGroupExpression(Macro.Parameter parameter, List<Expression.EExpressionBodyExpression> expressions, boolean requireSingleton) {
private void readGroupExpression(Macro.Parameter parameter, boolean requireSingleton) {
Macro.ParameterEncoding encoding = parameter.getType();
if (encoding == Macro.ParameterEncoding.Tagged) {
enterTaggedArgumentGroup();
Expand All @@ -1636,7 +1634,7 @@ private void readGroupExpression(Macro.Parameter parameter, List<Expression.EExp
expressions.add(Expression.Placeholder.INSTANCE);
boolean isSingleton = true;
while (nextGroupedValue() != Event.NEEDS_INSTRUCTION || isMacroInvocation()) {
readValueAsExpression(false, expressions);
readValueAsExpression(false);
isSingleton = false;
}
if (requireSingleton && !isSingleton) {
Expand All @@ -1654,36 +1652,35 @@ private void readGroupExpression(Macro.Parameter parameter, List<Expression.EExp

/**
* Adds an expression that conveys that the parameter was not present (void).
* @param expressions receives the expressions as they are materialized.
*/
private void addVoidExpression(List<Expression.EExpressionBodyExpression> 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<Expression.EExpressionBodyExpression> 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",
Expand All @@ -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.");
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/amazon/ion/impl/IonReaderTextSystemX.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1251,14 +1251,14 @@ private class TextEExpressionArgsReader extends EExpressionArgsReader {
}

@Override
protected void readParameter(Macro.Parameter parameter, long parameterPresence, List<Expression.EExpressionBodyExpression> 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
Expand Down
43 changes: 17 additions & 26 deletions src/main/java/com/amazon/ion/impl/macro/EExpressionArgsReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ public abstract class EExpressionArgsReader {

private final ReaderAdapter reader;

// Reusable sink for expressions.
protected final List<Expression.EExpressionBodyExpression> expressions = new ArrayList<>(16);

/**
* Constructor.
* @param reader the {@link ReaderAdapter} from which to read {@link Expression}s.
Expand Down Expand Up @@ -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<Expression.EExpressionBodyExpression> 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.
Expand All @@ -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<SymbolToken> annotations,
List<Expression.EExpressionBodyExpression> expressions
List<SymbolToken> annotations
) {
Expression.EExpressionBodyExpression expression;
if (reader.isNullValue()) {
Expand Down Expand Up @@ -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<SymbolToken> annotations,
List<Expression.EExpressionBodyExpression> expressions
List<SymbolToken> annotations
) {
int startIndex = expressions.size();
expressions.add(Expression.Placeholder.INSTANCE);
Expand All @@ -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
Expand Down Expand Up @@ -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<Expression.EExpressionBodyExpression> 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()));
}
Expand All @@ -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<Expression.EExpressionBodyExpression> 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<SymbolToken> 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<Expression.EExpressionBodyExpression> expressions) {
private void collectEExpressionArgs() {
Macro macro = loadMacro();
stepIntoEExpression();
List<Macro.Parameter> signature = macro.getSignature();
Expand All @@ -263,7 +256,6 @@ private void collectEExpressionArgs(List<Expression.EExpressionBodyExpression> e
readParameter(
signature.get(i),
presenceBitmap == null ? PresenceBitmap.EXPRESSION : presenceBitmap.get(i),
expressions,
i == (numberOfParameters - 1)
);
}
Expand All @@ -276,13 +268,12 @@ private void collectEExpressionArgs(List<Expression.EExpressionBodyExpression> e
* them to the macro evaluator.
*/
public void beginEvaluatingMacroInvocation(MacroEvaluator macroEvaluator) {
// TODO performance: use a pool of expression lists to avoid repetitive allocations.
List<Expression.EExpressionBodyExpression> 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);
}
}

0 comments on commit 8c121f7

Please sign in to comment.