Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds an Expression pool to reduce allocations. #1037

Open
wants to merge 2 commits into
base: ion-11-encoding
Choose a base branch
from
Open

Adds an Expression pool to reduce allocations. #1037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
403e648
Adds an Expression pool to reduce allocations.
tgregg Jan 17, 2025
8c64373
Do not copy when adding annotations to Expression instances; use an e…
tgregg Jan 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1647,15 +1647,15 @@ private void readGroupExpression(Macro.Parameter parameter, boolean requireSingl
if (exitArgumentGroup() == Event.NEEDS_DATA) {
throw new UnsupportedOperationException("TODO: support continuable parsing of macro arguments.");
}
expressions.set(startIndex, new Expression.ExpressionGroup(startIndex, expressions.size()));
expressions.set(startIndex, expressionPool.createExpressionGroup(startIndex, expressions.size()));
}

/**
* Adds an expression that conveys that the parameter was not present (void).
*/
private void addVoidExpression() {
int startIndex = expressions.size();
expressions.add(new Expression.ExpressionGroup(startIndex, startIndex + 1));
expressions.add(expressionPool.createExpressionGroup(startIndex, startIndex + 1));
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/amazon/ion/impl/IonReaderTextSystemX.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1255,7 +1255,7 @@ protected void readParameter(Macro.Parameter parameter, long parameterPresence,
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));
expressions.add(expressionPool.createExpressionGroup(index, index));
return;
}
readValueAsExpression(isTrailing && parameter.getCardinality().canBeMulti);
Expand Down
43 changes: 22 additions & 21 deletions src/main/java/com/amazon/ion/impl/macro/EExpressionArgsReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import com.amazon.ion.impl.bin.PresenceBitmap;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

/**
Expand All @@ -28,6 +27,8 @@ public abstract class EExpressionArgsReader {
// Reusable sink for expressions.
protected final List<Expression.EExpressionBodyExpression> expressions = new ArrayList<>(16);

protected final PooledExpressionFactory expressionPool = new PooledExpressionFactory();

/**
* Constructor.
* @param reader the {@link ReaderAdapter} from which to read {@link Expression}s.
Expand Down Expand Up @@ -113,45 +114,45 @@ private void readScalarValueAsExpression(
) {
Expression.EExpressionBodyExpression expression;
if (reader.isNullValue()) {
expression = new Expression.NullValue(annotations, type);
expression = expressionPool.createNullValue(annotations, type);
} else {
switch (type) {
case BOOL:
expression = new Expression.BoolValue(annotations, reader.booleanValue());
expression = expressionPool.createBoolValue(annotations, reader.booleanValue());
break;
case INT:
switch (reader.getIntegerSize()) {
case INT:
case LONG:
expression = new Expression.LongIntValue(annotations, reader.longValue());
expression = expressionPool.createLongIntValue(annotations, reader.longValue());
break;
case BIG_INTEGER:
expression = new Expression.BigIntValue(annotations, reader.bigIntegerValue());
expression = expressionPool.createBigIntValue(annotations, reader.bigIntegerValue());
break;
default:
throw new IllegalStateException();
}
break;
case FLOAT:
expression = new Expression.FloatValue(annotations, reader.doubleValue());
expression = expressionPool.createFloatValue(annotations, reader.doubleValue());
break;
case DECIMAL:
expression = new Expression.DecimalValue(annotations, reader.decimalValue());
expression = expressionPool.createDecimalValue(annotations, reader.decimalValue());
break;
case TIMESTAMP:
expression = new Expression.TimestampValue(annotations, reader.timestampValue());
expression = expressionPool.createTimestampValue(annotations, reader.timestampValue());
break;
case SYMBOL:
expression = new Expression.SymbolValue(annotations, reader.symbolValue());
expression = expressionPool.createSymbolValue(annotations, reader.symbolValue());
break;
case STRING:
expression = new Expression.StringValue(annotations, reader.stringValue());
expression = expressionPool.createStringValue(annotations, reader.stringValue());
break;
case CLOB:
expression = new Expression.ClobValue(annotations, reader.newBytes());
expression = expressionPool.createClobValue(annotations, reader.newBytes());
break;
case BLOB:
expression = new Expression.BlobValue(annotations, reader.newBytes());
expression = expressionPool.createBlobValue(annotations, reader.newBytes());
break;
default:
throw new IllegalStateException();
Expand All @@ -177,7 +178,7 @@ private void readContainerValueAsExpression(
stepInRaw();
while (nextRaw()) {
if (type == IonType.STRUCT) {
expressions.add(new Expression.FieldName(reader.getFieldNameSymbol()));
expressions.add(expressionPool.createFieldName(reader.getFieldNameSymbol()));
}
readValueAsExpression(false); // TODO avoid recursion
}
Expand All @@ -186,18 +187,17 @@ private void readContainerValueAsExpression(
// start and end indices of its expressions.
Expression.EExpressionBodyExpression expression;
if (isExpressionGroup) {
expression = new Expression.ExpressionGroup(startIndex, expressions.size());
expression = expressionPool.createExpressionGroup(startIndex, expressions.size());
} else {
switch (type) {
case LIST:
expression = new Expression.ListValue(annotations, startIndex, expressions.size());
expression = expressionPool.createListValue(annotations, startIndex, expressions.size());
break;
case SEXP:
expression = new Expression.SExpValue(annotations, startIndex, expressions.size());
expression = expressionPool.createSExpValue(annotations, startIndex, expressions.size());
break;
case STRUCT:
// TODO consider whether templateStructIndex could be leveraged or should be removed
expression = new Expression.StructValue(annotations, startIndex, expressions.size(), Collections.emptyMap());
expression = expressionPool.createStructValue(annotations, startIndex, expressions.size());
break;
default:
throw new IllegalStateException();
Expand All @@ -215,7 +215,7 @@ private void readStreamAsExpressionGroup() {
do {
readValueAsExpression(false); // TODO avoid recursion
} while (nextRaw());
expressions.set(startIndex, new Expression.ExpressionGroup(startIndex, expressions.size()));
expressions.set(startIndex, expressionPool.createExpressionGroup(startIndex, expressions.size()));
}

/**
Expand Down Expand Up @@ -260,7 +260,7 @@ private void collectEExpressionArgs() {
);
}
stepOutOfEExpression();
expressions.set(invocationStartIndex, new Expression.EExpression(macro, invocationStartIndex, expressions.size()));
expressions.set(invocationStartIndex, expressionPool.createEExpression(macro, invocationStartIndex, expressions.size()));
}

/**
Expand All @@ -269,9 +269,10 @@ private void collectEExpressionArgs() {
*/
public void beginEvaluatingMacroInvocation(MacroEvaluator macroEvaluator) {
expressions.clear();
expressionPool.clear();
// TODO performance: avoid fully materializing all expressions up-front.
if (reader.isInStruct()) {
expressions.add(new Expression.FieldName(reader.getFieldNameSymbol()));
expressions.add(expressionPool.createFieldName(reader.getFieldNameSymbol()));
}
collectEExpressionArgs();
macroEvaluator.initExpansion(expressions);
Expand Down
Loading
Loading