Skip to content

Commit 25ce9d0

Browse files
committed
Avoid creating child ExpansionInfos for each container during evaluation.
1 parent 8e6de67 commit 25ce9d0

File tree

3 files changed

+13
-40
lines changed

3 files changed

+13
-40
lines changed

src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1652,7 +1652,7 @@ private void addVoidExpression() {
16521652
}
16531653

16541654
@Override
1655-
protected void readParameter(Macro.Parameter parameter, long parameterPresence, boolean isTrailing) {
1655+
protected void readParameter(Macro.Parameter parameter, long parameterPresence) {
16561656
switch (parameter.getCardinality()) {
16571657
case ZeroOrOne:
16581658
if (parameterPresence == PresenceBitmap.EXPRESSION) {

src/main/java/com/amazon/ion/impl/LazyEExpressionArgsReader.java

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
// SPDX-License-Identifier: Apache-2.0
33
package com.amazon.ion.impl;
44

5-
import com.amazon.ion.IonCursor;
6-
import com.amazon.ion.IonException;
75
import com.amazon.ion.IonType;
86
import com.amazon.ion.MacroAwareIonWriter;
97
import com.amazon.ion.SymbolToken;
@@ -19,9 +17,7 @@
1917

2018
import java.io.IOException;
2119
import java.util.Arrays;
22-
import java.util.HashMap;
2320
import java.util.List;
24-
import java.util.Map;
2521

2622
/**
2723
* An {@link LazyEExpressionArgsReader} reads an E-Expression from a {@link ReaderAdapter}, constructs
@@ -139,9 +135,8 @@ private Marker getMarker(int index) {
139135
* Reads a single parameter to a macro invocation.
140136
* @param parameter information about the parameter from the macro signature.
141137
* @param parameterPresence the presence bits dedicated to this parameter (unused in text).
142-
* @param isTrailing true if this parameter is the last one in the signature; otherwise, false (unused in binary).
143138
*/
144-
protected abstract void readParameter(Macro.Parameter parameter, long parameterPresence, boolean isTrailing);
139+
protected abstract void readParameter(Macro.Parameter parameter, long parameterPresence);
145140

146141
/**
147142
* Reads the macro's address and attempts to resolve that address to a Macro from the macro table.
@@ -231,7 +226,7 @@ protected void readValueAsExpression(boolean isImplicitRest) {
231226
}
232227
}
233228

234-
private int collectAndFlattenComplexEExpression(int numberOfParameters, ExpressionTape.Core macroBodyTape, List<Macro.Parameter> signature, PresenceBitmap presenceBitmap, int markerPoolStartIndex) {
229+
private int collectAndFlattenComplexEExpression(ExpressionTape.Core macroBodyTape, List<Macro.Parameter> signature, PresenceBitmap presenceBitmap, int markerPoolStartIndex) {
235230
// TODO drop expression groups? No longer needed after the variables are resolved. Further simplifies the evaluator. Might miss validation? Also, would need to distribute field names over the elements of the group.
236231
int macroBodyTapeIndex = 0;
237232
int numberOfVariables = macroBodyTape.getNumberOfVariables();
@@ -250,8 +245,7 @@ private int collectAndFlattenComplexEExpression(int numberOfParameters, Expressi
250245
int startIndexInTape = expressionTape.size();
251246
readParameter(
252247
signature.get(invocationOrdinal),
253-
presenceBitmap == null ? PresenceBitmap.EXPRESSION : presenceBitmap.get(invocationOrdinal),
254-
invocationOrdinal == (numberOfParameters - 1)
248+
presenceBitmap == null ? PresenceBitmap.EXPRESSION : presenceBitmap.get(invocationOrdinal)
255249
);
256250
Marker marker = getMarker(markerPoolStartIndex + invocationOrdinal);
257251
marker.typeId = null;
@@ -264,8 +258,7 @@ private int collectAndFlattenComplexEExpression(int numberOfParameters, Expressi
264258
expressionTape = expressionTapeScratch;
265259
readParameter(
266260
signature.get(invocationOrdinal),
267-
presenceBitmap == null ? PresenceBitmap.EXPRESSION : presenceBitmap.get(invocationOrdinal),
268-
invocationOrdinal == (numberOfParameters - 1)
261+
presenceBitmap == null ? PresenceBitmap.EXPRESSION : presenceBitmap.get(invocationOrdinal)
269262
);
270263
expressionTape = expressionTapeOrdered;
271264
Marker marker = getMarker(markerPoolStartIndex + invocationOrdinal);
@@ -293,7 +286,7 @@ private int collectAndFlattenComplexEExpression(int numberOfParameters, Expressi
293286
return macroBodyTapeIndex;
294287
}
295288

296-
private int collectAndFlattenSimpleEExpression(int numberOfParameters, ExpressionTape.Core macroBodyTape, List<Macro.Parameter> signature, PresenceBitmap presenceBitmap) {
289+
private int collectAndFlattenSimpleEExpression(ExpressionTape.Core macroBodyTape, List<Macro.Parameter> signature, PresenceBitmap presenceBitmap) {
297290
// TODO drop expression groups? No longer needed after the variables are resolved. Further simplifies the evaluator. Might miss validation? Also, would need to distribute field names over the elements of the group.
298291
int macroBodyTapeIndex = 0;
299292
int numberOfVariables = macroBodyTape.getNumberOfVariables();
@@ -304,8 +297,7 @@ private int collectAndFlattenSimpleEExpression(int numberOfParameters, Expressio
304297
// variable in the signature. This means it can be copied into the tape without using scratch space.
305298
readParameter(
306299
signature.get(i),
307-
presenceBitmap == null ? PresenceBitmap.EXPRESSION : presenceBitmap.get(i),
308-
i == (numberOfParameters - 1)
300+
presenceBitmap == null ? PresenceBitmap.EXPRESSION : presenceBitmap.get(i)
309301
);
310302
}
311303
return macroBodyTapeIndex;
@@ -324,9 +316,9 @@ private ExpressionTape collectAndFlattenUserEExpressionArgs(boolean isTopLevel,
324316
return constantTape;
325317
}
326318
} else if (macro.isSimple()) {
327-
macroBodyTapeIndex = collectAndFlattenSimpleEExpression(numberOfParameters, macroBodyTape, signature, presenceBitmap);
319+
macroBodyTapeIndex = collectAndFlattenSimpleEExpression(macroBodyTape, signature, presenceBitmap);
328320
} else {
329-
macroBodyTapeIndex = collectAndFlattenComplexEExpression(numberOfParameters, macroBodyTape, signature, presenceBitmap, markerPoolIndex + 1);
321+
macroBodyTapeIndex = collectAndFlattenComplexEExpression(macroBodyTape, signature, presenceBitmap, markerPoolIndex + 1);
330322
}
331323
// Copy everything after the last parameter.
332324
expressionTape.copyFromRange(macroBodyTape, macroBodyTapeIndex, macroBodyTape.size());
@@ -341,8 +333,7 @@ private void collectVerbatimEExpressionArgs(Macro macro, List<Macro.Parameter> s
341333
for (int i = 0; i < numberOfParameters; i++) {
342334
readParameter(
343335
signature.get(i),
344-
presenceBitmap == null ? PresenceBitmap.EXPRESSION : presenceBitmap.get(i),
345-
i == (numberOfParameters - 1)
336+
presenceBitmap == null ? PresenceBitmap.EXPRESSION : presenceBitmap.get(i)
346337
);
347338
}
348339
stepOutOfEExpression();

src/main/java/com/amazon/ion/impl/macro/LazyMacroEvaluator.kt

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ class LazyMacroEvaluator : IonReader {
523523
expressionTape = arguments
524524
sideEffectExpander = getExpander(EMPTY, sideEffects)
525525
sideEffectExpander!!.keepAlive = true
526-
currentExpander = null
526+
currentExpander = getExpander(STREAM, expressionTape)
527527
currentFieldName = fieldName
528528
currentAnnotations = null
529529
eExpressionIndex = -1;
@@ -533,20 +533,15 @@ class LazyMacroEvaluator : IonReader {
533533
/**
534534
* A container in the macro evaluator's [containerStack].
535535
*/
536-
private data class ContainerInfo(var type: Type = Type.Uninitialized, private var _expansion: ExpansionInfo? = null) {
536+
private data class ContainerInfo(var type: Type = Type.Uninitialized) {
537537
enum class Type { TopLevel, List, Sexp, Struct, Uninitialized }
538538

539539
fun close() {
540-
_expansion?.close()
541-
_expansion = null
542540
currentFieldName = null
543541
container = null
544542
type = Type.Uninitialized
545543
}
546544

547-
var expansion: ExpansionInfo
548-
get() = _expansion!!
549-
set(value) { _expansion = value }
550545
@JvmField var currentFieldName: SymbolToken? = null
551546
@JvmField var container: IonType? = null
552547
}
@@ -594,11 +589,6 @@ class LazyMacroEvaluator : IonReader {
594589

595590
var parentExpansion: ExpansionInfo? = null
596591

597-
/**
598-
* Gets the [ExpansionInfo] at the top of the stack of [childExpansion]s.
599-
*/
600-
fun top(): ExpansionInfo = childExpansion?.top() ?: this
601-
602592
/**
603593
* Returns an expansion for the given variable.
604594
*/
@@ -734,8 +724,6 @@ class LazyMacroEvaluator : IonReader {
734724
session.reset(fieldName, encodingExpressions)
735725
val ci = containerStack.push { _ -> }
736726
ci.type = ContainerInfo.Type.TopLevel
737-
738-
ci.expansion = session.getExpander(STREAM, session.expressionTape)
739727
}
740728

741729
override fun next(): IonType? {
@@ -779,8 +767,7 @@ class LazyMacroEvaluator : IonReader {
779767
if (containerStack.size() <= 1) throw IonException("Nothing to step out of.")
780768
val popped = containerStack.pop()
781769
val currentContainer = containerStack.peek()
782-
session.currentExpander = currentContainer.expansion.top()
783-
popped.expansion.tape!!.advanceToAfterEndContainer()
770+
session.expressionTape!!.advanceToAfterEndContainer()
784771
popped.close()
785772
session.currentFieldName = currentContainer.currentFieldName
786773
currentValueType = null // Must call `next()` to get the next value
@@ -804,11 +791,6 @@ class LazyMacroEvaluator : IonReader {
804791
IonType.STRUCT -> ContainerInfo.Type.Struct
805792
else -> unreachable()
806793
}
807-
// TODO can this expander be eliminated?
808-
ci.expansion = session.getExpander(
809-
expansionKind = STREAM,
810-
tape = session.expressionTape
811-
)
812794
ci.currentFieldName = null
813795
currentExpr = null
814796
session.currentFieldName = null

0 commit comments

Comments
 (0)