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

Allows the binary reader to avoid allocating PresenceBitmaps when a macro signature has less than 5 required parameters. #1032

Merged

Conversation

tgregg
Copy link
Contributor

@tgregg tgregg commented Jan 16, 2025

Description of changes:
This reduces allocations for these common signature shapes. We can expect to see additional improvements using pooling/caching and/or precomputation for known signatures.

Before: 271 ms/op
After: 264 ms/op (-2.6%)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…acro signature has less than 5 required parameters.
@tgregg tgregg requested a review from popematt January 16, 2025 00:50
Copy link
Contributor

@popematt popematt left a comment

Choose a reason for hiding this comment

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

I think in the long run, the existing abstraction may work for the writer, but for the reader we need something that is implemented a bit more like an iterator (and can be pooled). Something like this, I think:

class PresenceBitsReader {

    var buffer: ByteArray = emptyArray()
    var startInclusive: Int = 0
    var endExclusive: Int = 0
    // Tracks pairs of bits rather than bytes.
    var i: Int = 0

    fun init(buffer: ByteArray, startInclusive: Int, endExclusive: Int) {
        i = 0
        // ...copy the rest of the fields in, too
    }

    fun hasNext() = i / 4 < endExclusive

    // Returns the next raw AEB value: 0, 1, or 2
    fun next(): Int {
        val byte = buffer[i / 4]
        val bit_position = i mod 4
        i++
        return when (bit_position) {
            0 -> (byte and 0b00000011)
            1 -> (byte and 0b00001100) ushr 2
            2 -> (byte and 0b00110000) ushr 4
            3 -> (byte and 0b11000000) ushr 6
            else -> throw IllegalStateException("Unreachable")
        }
    }
}

@tgregg tgregg merged commit 50ccac0 into ion-11-encoding Jan 16, 2025
17 checks passed
@tgregg tgregg deleted the ion-11-encoding-optimize-common-presence-bitmaps branch January 16, 2025 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants