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.

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 a pool for PresenceBitmap instances. #1044

Open
wants to merge 1 commit into
base: ion-11-encoding-optimize-initial-expression-array-size-session-pools
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 12 additions & 4 deletions src/main/java/com/amazon/ion/impl/IonCursorBinary.java
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,13 @@ private static class ArgumentGroupMarker {
*/
TaglessEncoding taglessType = null;

/**
* A pool for PresenceBitmap instances that can be scoped to the current value. This is used when skipping macro
* invocations. PresenceBitmap instances that need to stay in scope after the cursor moves to the next value
* cannot come from this pool; an externally managed pool must be used in that case.
*/
private final PresenceBitmap.Companion.PooledFactory presenceBitmapPool = new PresenceBitmap.Companion.PooledFactory();

/**
* @return the given configuration's DataHandler, or null if that DataHandler is a no-op.
*/
Expand Down Expand Up @@ -1907,12 +1914,12 @@ private boolean slowIsDelimitedEnd_1_1() {
* Loads the presence bitmap for the given signature. Upon calling this method, `peekIndex` must point to the
* first byte in the argument encoding bitmap (if applicable for the signature).
* @param signature the signature of the macro invocation for which to read a presence bitmap.
* @param pool the PresenceBitmap pool to use.
* @return the presence bitmap.
*/
protected PresenceBitmap loadPresenceBitmap(List<Macro.Parameter> signature) {
// TODO performance: reuse or pool the presence bitmaps.
protected PresenceBitmap loadPresenceBitmap(List<Macro.Parameter> signature, PresenceBitmap.Companion.PooledFactory pool) {
// Note: when there is no AEB, the following initializes the presence bitmap to "EXPRESSION" for each parameter.
PresenceBitmap presenceBitmap = PresenceBitmap.create(signature);
PresenceBitmap presenceBitmap = PresenceBitmap.create(signature, pool);
if (presenceBitmap.getByteSize() > 0) {
if (fillArgumentEncodingBitmap(presenceBitmap.getByteSize()) == IonCursor.Event.NEEDS_DATA) {
throw new UnsupportedOperationException("TODO: support continuable parsing of AEBs.");
Expand Down Expand Up @@ -2041,7 +2048,7 @@ private boolean skipMacroInvocation() {
}
stepIntoEExpression();
List<Macro.Parameter > signature = macro.getSignature();
PresenceBitmap presenceBitmap = loadPresenceBitmap(signature);
PresenceBitmap presenceBitmap = loadPresenceBitmap(signature, presenceBitmapPool);
for (int i = 0; i < signature.size(); i++) {
Macro.Parameter parameter = signature.get(i);
long parameterPresence = presenceBitmap.get(i);
Expand Down Expand Up @@ -2335,6 +2342,7 @@ private void reset() {
macroInvocationId = -1;
isSystemInvocation = false;
taglessType = null;
presenceBitmapPool.clear();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1727,7 +1727,7 @@ protected Macro loadMacro() {

@Override
protected PresenceBitmap loadPresenceBitmapIfNecessary(List<Macro.Parameter> signature) {
return IonReaderContinuableCoreBinary.this.loadPresenceBitmap(signature);
return IonReaderContinuableCoreBinary.this.loadPresenceBitmap(signature, presenceBitmapPool);
}

@Override
Expand Down
76 changes: 64 additions & 12 deletions src/main/java/com/amazon/ion/impl/bin/PresenceBitmap.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@
package com.amazon.ion.impl.bin

import com.amazon.ion.*
import com.amazon.ion.impl.bin.PresenceBitmap.Companion.EXPRESSION
import com.amazon.ion.impl.bin.PresenceBitmap.Companion.GROUP
import com.amazon.ion.impl.bin.PresenceBitmap.Companion.VOID
import com.amazon.ion.impl.macro.*
import com.amazon.ion.impl.macro.Macro.*
import com.amazon.ion.impl.macro.MacroEvaluator.*
import java.util.*

/**
* Utility class for setting, storing, reading, and writing presence bits.
Expand Down Expand Up @@ -70,16 +75,40 @@ internal class PresenceBitmap(

const val MAX_SUPPORTED_PARAMETERS = PB_SLOTS_PER_LONG * 4

private val ZERO_PARAMETERS: PresenceBitmap = allRequired(0)
private val ONE_REQUIRED_PARAMETER: PresenceBitmap = allRequired(1)
private val TWO_REQUIRED_PARAMETERS: PresenceBitmap = allRequired(2)
private val THREE_REQUIRED_PARAMETERS: PresenceBitmap = allRequired(3)
private val FOUR_REQUIRED_PARAMETERS: PresenceBitmap = allRequired(4)
private val ZERO_PARAMETERS: PresenceBitmap = allRequired(0) { PresenceBitmap() }
private val ONE_REQUIRED_PARAMETER: PresenceBitmap = allRequired(1) { PresenceBitmap() }
private val TWO_REQUIRED_PARAMETERS: PresenceBitmap = allRequired(2) { PresenceBitmap() }
private val THREE_REQUIRED_PARAMETERS: PresenceBitmap = allRequired(3) { PresenceBitmap() }
private val FOUR_REQUIRED_PARAMETERS: PresenceBitmap = allRequired(4) { PresenceBitmap() }

/** Pool for PresenceBitmap instances. */
class PooledFactory {

private var index = 0

private var pool: Array<PresenceBitmap?> = Array(32) { null }

/** Gets an instance from the pool, allocating a new one only if necessary. The returned instance must be reset. */
fun get(): PresenceBitmap {
if (index >= pool.size) {
pool = pool.copyOf(pool.size * 2)
}
if (pool[index] == null) {
pool[index] = PresenceBitmap()
}
return pool[index++]!!
}

/** Clears the pool. Calling this method invalidates all instances previously returned by [get]. */
fun clear() {
index = 0
}
}

/** Creates a PresenceBitmap for the given number of required parameters */
private fun allRequired(numberOfParameters: Int): PresenceBitmap {
private inline fun allRequired(numberOfParameters: Int, supplier: () -> PresenceBitmap): PresenceBitmap {
if (numberOfParameters > MAX_SUPPORTED_PARAMETERS) throw IonException("Macros with more than 128 parameters are not supported by this implementation.")
val bitmap = PresenceBitmap(emptyList(), 0, numberOfParameters)
val bitmap = supplier().reset(emptyList(), 0, numberOfParameters)
for (i in 0 until numberOfParameters) {
bitmap.setUnchecked(i, EXPRESSION)
}
Expand All @@ -88,22 +117,30 @@ internal class PresenceBitmap(

/** Creates or reuses a [PresenceBitmap] for the given signature. */
@JvmStatic
fun create(signature: List<Parameter>): PresenceBitmap {
fun create(signature: List<Parameter>, pool: PooledFactory): PresenceBitmap {
if (signature.size > MAX_SUPPORTED_PARAMETERS) throw IonException("Macros with more than 128 parameters are not supported by this implementation.")
// Calculate the actual number of presence bits that will be encoded for the given signature.
val nonRequiredParametersCount = signature.count { it.cardinality != ParameterCardinality.ExactlyOne }
val usePresenceBits = nonRequiredParametersCount > PRESENCE_BITS_SIZE_THRESHOLD || signature.any { it.type.taglessEncodingKind != null }
var nonRequiredParametersCount = 0
var usePresenceBits = false
for (i in signature.indices) {
val parameter = signature[i]
if (parameter.cardinality != ParameterCardinality.ExactlyOne) {
nonRequiredParametersCount++
}
usePresenceBits = usePresenceBits or (parameter.type.taglessEncodingKind != null)
}
usePresenceBits = usePresenceBits or (nonRequiredParametersCount > PRESENCE_BITS_SIZE_THRESHOLD)
Comment on lines -94 to +132
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The result is the same, but the new (uglier) version achieves two things:

  1. It eliminates ArrayList$Itr allocations that are implicit to Kotlin's List.count and List.any methods. These were showing up prominently in memory profiles. This change alone lowered allocation rate from 147 KB/op to 139 KB/op.
  2. It only loops through the signature once, instead of (worst case) two full times.

Comment on lines +130 to +132
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or spelled out does not short-circuit, is there a reason to use it?

Suggested change
usePresenceBits = usePresenceBits or (parameter.type.taglessEncodingKind != null)
}
usePresenceBits = usePresenceBits or (nonRequiredParametersCount > PRESENCE_BITS_SIZE_THRESHOLD)
usePresenceBits = usePresenceBits || (parameter.type.taglessEncodingKind != null)
}
usePresenceBits = usePresenceBits || (nonRequiredParametersCount > PRESENCE_BITS_SIZE_THRESHOLD)

val size = if (usePresenceBits) nonRequiredParametersCount else 0
return if (size > 0) {
PresenceBitmap(signature, nonRequiredParametersCount, signature.size)
pool.get().reset(signature, nonRequiredParametersCount, signature.size)
} else {
when (signature.size) {
0 -> ZERO_PARAMETERS
1 -> ONE_REQUIRED_PARAMETER
2 -> TWO_REQUIRED_PARAMETERS
3 -> THREE_REQUIRED_PARAMETERS
4 -> FOUR_REQUIRED_PARAMETERS
else -> allRequired(signature.size)
else -> allRequired(signature.size, pool::get)
}
}
}
Expand All @@ -122,6 +159,21 @@ internal class PresenceBitmap(
val byteSize: Int
get() = size divideByRoundingUp PB_SLOTS_PER_BYTE

/**
* Resets this [PresenceBitmap] for the given signature, size, and parameter count. After this method is called,
* callers must set the bitmap as necessary, such as by calling [readFrom].
*/
fun reset(signature: List<Parameter>, size: Int, totalParameterCount: Int): PresenceBitmap {
this.signature = signature
this.size = size
this.totalParameterCount = totalParameterCount
a = 0
b = 0
c = 0
d = 0
return this
}

/** Resets this [PresenceBitmap] for the given signature. */
fun initialize(signature: List<Parameter>) {
if (signature.size > MAX_SUPPORTED_PARAMETERS) throw IonException("Macros with more than 128 parameters are not supported by this implementation.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ public abstract class EExpressionArgsReader {

protected final PooledExpressionFactory expressionPool = new PooledExpressionFactory();

protected final PresenceBitmap.Companion.PooledFactory presenceBitmapPool = new PresenceBitmap.Companion.PooledFactory();

/**
* Constructor.
* @param reader the {@link ReaderAdapter} from which to read {@link Expression}s.
Expand Down Expand Up @@ -270,6 +272,7 @@ private void collectEExpressionArgs() {
public void beginEvaluatingMacroInvocation(MacroEvaluator macroEvaluator) {
expressions.clear();
expressionPool.clear();
presenceBitmapPool.clear();
// TODO performance: avoid fully materializing all expressions up-front.
if (reader.isInStruct()) {
expressions.add(expressionPool.createFieldName(reader.getFieldNameSymbol()));
Expand Down