Skip to content

Commit

Permalink
Adds an Expression pool to reduce allocations.
Browse files Browse the repository at this point in the history
  • Loading branch information
tgregg committed Jan 17, 2025
1 parent cb37a5c commit ffcd659
Show file tree
Hide file tree
Showing 5 changed files with 437 additions and 56 deletions.
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
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
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 @@ -29,6 +28,8 @@ public abstract class EExpressionArgsReader {
// e-expression invocations.
protected final List<Expression.EExpressionBodyExpression> expressions = new ArrayList<>(64);

protected final PooledExpressionFactory expressionPool = new PooledExpressionFactory();

/**
* Constructor.
* @param reader the {@link ReaderAdapter} from which to read {@link Expression}s.
Expand Down Expand Up @@ -114,45 +115,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 @@ -178,7 +179,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 @@ -187,18 +188,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 @@ -216,7 +216,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 @@ -261,7 +261,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 @@ -270,9 +270,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
64 changes: 32 additions & 32 deletions src/main/java/com/amazon/ion/impl/macro/Expression.kt
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ sealed interface Expression {
* The position of this expression in its containing list.
* Child expressions (if any) start at `selfIndex + 1`.
*/
val selfIndex: Int
var selfIndex: Int
/**
* The index of the first child expression (if any).
* Always equal to `selfIndex + 1`.
Expand All @@ -38,7 +38,7 @@ sealed interface Expression {
* The exclusive end of the child expressions (if any).
* If there are no child expressions, will be equal to [startInclusive].
*/
val endExclusive: Int
var endExclusive: Int
}

/** Marker interface representing expressions that can be present in E-Expressions. */
Expand Down Expand Up @@ -72,7 +72,7 @@ sealed interface Expression {
* Interface for expressions that are _values_ in the Ion data model.
*/
sealed interface DataModelValue : DataModelExpression {
val annotations: List<SymbolToken>
var annotations: List<SymbolToken>
val type: IonType

fun withAnnotations(annotations: List<SymbolToken>): DataModelValue
Expand All @@ -97,14 +97,14 @@ sealed interface Expression {
* @property selfIndex the index of the first expression of the expression group (i.e. this instance)
* @property endExclusive the index of the last expression contained in the expression group
*/
data class ExpressionGroup(override val selfIndex: Int, override val endExclusive: Int) : EExpressionBodyExpression, TemplateBodyExpression, HasStartAndEnd
data class ExpressionGroup(override var selfIndex: Int, override var endExclusive: Int) : EExpressionBodyExpression, TemplateBodyExpression, HasStartAndEnd

// Scalars
data class NullValue(override val annotations: List<SymbolToken> = emptyList(), override val type: IonType) : DataModelValue {
data class NullValue(override var annotations: List<SymbolToken> = emptyList(), override var type: IonType) : DataModelValue {
override fun withAnnotations(annotations: List<SymbolToken>) = copy(annotations = annotations)
}

data class BoolValue(override val annotations: List<SymbolToken> = emptyList(), val value: Boolean) : DataModelValue {
data class BoolValue(override var annotations: List<SymbolToken> = emptyList(), var value: Boolean) : DataModelValue {
override val type: IonType get() = IonType.BOOL
override fun withAnnotations(annotations: List<SymbolToken>) = copy(annotations = annotations)
}
Expand All @@ -114,31 +114,31 @@ sealed interface Expression {
val longValue: Long
}

data class LongIntValue(override val annotations: List<SymbolToken> = emptyList(), val value: Long) : IntValue {
data class LongIntValue(override var annotations: List<SymbolToken> = emptyList(), var value: Long) : IntValue {
override val type: IonType get() = IonType.INT
override fun withAnnotations(annotations: List<SymbolToken>) = copy(annotations = annotations)
override val bigIntegerValue: BigInteger get() = BigInteger.valueOf(value)
override val longValue: Long get() = value
}

data class BigIntValue(override val annotations: List<SymbolToken> = emptyList(), val value: BigInteger) : IntValue {
data class BigIntValue(override var annotations: List<SymbolToken> = emptyList(), var value: BigInteger) : IntValue {
override val type: IonType get() = IonType.INT
override fun withAnnotations(annotations: List<SymbolToken>) = copy(annotations = annotations)
override val bigIntegerValue: BigInteger get() = value
override val longValue: Long get() = value.longValueExact()
}

data class FloatValue(override val annotations: List<SymbolToken> = emptyList(), val value: Double) : DataModelValue {
data class FloatValue(override var annotations: List<SymbolToken> = emptyList(), var value: Double) : DataModelValue {
override val type: IonType get() = IonType.FLOAT
override fun withAnnotations(annotations: List<SymbolToken>) = copy(annotations = annotations)
}

data class DecimalValue(override val annotations: List<SymbolToken> = emptyList(), val value: BigDecimal) : DataModelValue {
data class DecimalValue(override var annotations: List<SymbolToken> = emptyList(), var value: BigDecimal) : DataModelValue {
override val type: IonType get() = IonType.DECIMAL
override fun withAnnotations(annotations: List<SymbolToken>) = copy(annotations = annotations)
}

data class TimestampValue(override val annotations: List<SymbolToken> = emptyList(), val value: Timestamp) : DataModelValue {
data class TimestampValue(override var annotations: List<SymbolToken> = emptyList(), var value: Timestamp) : DataModelValue {
override val type: IonType get() = IonType.TIMESTAMP
override fun withAnnotations(annotations: List<SymbolToken>) = copy(annotations = annotations)
}
Expand All @@ -147,13 +147,13 @@ sealed interface Expression {
val stringValue: String
}

data class StringValue(override val annotations: List<SymbolToken> = emptyList(), val value: String) : TextValue {
data class StringValue(override var annotations: List<SymbolToken> = emptyList(), var value: String) : TextValue {
override val type: IonType get() = IonType.STRING
override val stringValue: String get() = value
override fun withAnnotations(annotations: List<SymbolToken>) = copy(annotations = annotations)
}

data class SymbolValue(override val annotations: List<SymbolToken> = emptyList(), val value: SymbolToken) : TextValue {
data class SymbolValue(override var annotations: List<SymbolToken> = emptyList(), var value: SymbolToken) : TextValue {
override val type: IonType get() = IonType.SYMBOL
override val stringValue: String get() = value.assumeText()
override fun withAnnotations(annotations: List<SymbolToken>) = copy(annotations = annotations)
Expand All @@ -166,7 +166,7 @@ sealed interface Expression {
}

// We must override hashcode and equals in the lob types because `value` is a `byte[]`
data class BlobValue(override val annotations: List<SymbolToken> = emptyList(), override val value: ByteArray) : LobValue {
data class BlobValue(override var annotations: List<SymbolToken> = emptyList(), override var value: ByteArray) : LobValue {
override val type: IonType get() = IonType.BLOB
override fun withAnnotations(annotations: List<SymbolToken>) = copy(annotations = annotations)
override fun hashCode(): Int = annotations.hashCode() * 31 + value.contentHashCode()
Expand All @@ -178,7 +178,7 @@ sealed interface Expression {
}
}

data class ClobValue(override val annotations: List<SymbolToken> = emptyList(), override val value: ByteArray) : LobValue {
data class ClobValue(override var annotations: List<SymbolToken> = emptyList(), override var value: ByteArray) : LobValue {
override val type: IonType get() = IonType.CLOB
override fun withAnnotations(annotations: List<SymbolToken>) = copy(annotations = annotations)
override fun hashCode(): Int = annotations.hashCode() * 31 + value.contentHashCode()
Expand All @@ -197,9 +197,9 @@ sealed interface Expression {
* @property endExclusive the index of the last expression contained in the list
*/
data class ListValue(
override val annotations: List<SymbolToken> = emptyList(),
override val selfIndex: Int,
override val endExclusive: Int
override var annotations: List<SymbolToken> = emptyList(),
override var selfIndex: Int,
override var endExclusive: Int
) : DataModelContainer {
override val type: IonType get() = IonType.LIST
override fun withAnnotations(annotations: List<SymbolToken>) = copy(annotations = annotations)
Expand All @@ -209,9 +209,9 @@ sealed interface Expression {
* An Ion SExp that could contain variables or macro invocations.
*/
data class SExpValue(
override val annotations: List<SymbolToken> = emptyList(),
override val selfIndex: Int,
override val endExclusive: Int
override var annotations: List<SymbolToken> = emptyList(),
override var selfIndex: Int,
override var endExclusive: Int
) : DataModelContainer {
override val type: IonType get() = IonType.SEXP
override fun withAnnotations(annotations: List<SymbolToken>) = copy(annotations = annotations)
Expand All @@ -221,21 +221,21 @@ sealed interface Expression {
* An Ion Struct that could contain variables or macro invocations.
*/
data class StructValue(
override val annotations: List<SymbolToken> = emptyList(),
override val selfIndex: Int,
override val endExclusive: Int,
override var annotations: List<SymbolToken> = emptyList(),
override var selfIndex: Int,
override var endExclusive: Int,
val templateStructIndex: Map<String, List<Int>>
) : DataModelContainer {
override val type: IonType get() = IonType.STRUCT
override fun withAnnotations(annotations: List<SymbolToken>) = copy(annotations = annotations)
}

data class FieldName(val value: SymbolToken) : DataModelExpression
data class FieldName(var value: SymbolToken) : DataModelExpression

/**
* A reference to a variable that needs to be expanded.
*/
data class VariableRef(val signatureIndex: Int) : TemplateBodyExpression
data class VariableRef(var signatureIndex: Int) : TemplateBodyExpression

sealed interface InvokableExpression : HasStartAndEnd, Expression {
val macro: Macro
Expand All @@ -245,17 +245,17 @@ sealed interface Expression {
* A macro invocation that needs to be expanded.
*/
data class MacroInvocation(
override val macro: Macro,
override val selfIndex: Int,
override val endExclusive: Int
override var macro: Macro,
override var selfIndex: Int,
override var endExclusive: Int
) : TemplateBodyExpression, HasStartAndEnd, InvokableExpression

/**
* An e-expression that needs to be expanded.
*/
data class EExpression(
override val macro: Macro,
override val selfIndex: Int,
override val endExclusive: Int
override var macro: Macro,
override var selfIndex: Int,
override var endExclusive: Int
) : EExpressionBodyExpression, HasStartAndEnd, InvokableExpression
}
Loading

0 comments on commit ffcd659

Please sign in to comment.