Initial implementation of Ion 1.1 raw binary writer#660
Initial implementation of Ion 1.1 raw binary writer#660popematt merged 3 commits intoamazon-ion:ion-11-encodingfrom
Conversation
| expected, actual, | ||
| "Bytes don't match!\nEXPECTED:\n" + hexDump(expected) + "\nACTUAL:\n" + hexDump(actual) + "\n" |
There was a problem hiding this comment.
This is because of the change from org.junit.Assert to org.junit.jupiter.api.Assertions.
| fun writeFlexIntOrUIntInto(data: ByteArray, offset: Int, value: Long, numBytes: Int) { | ||
| var i = offset | ||
|
|
||
| when (numBytes) { |
There was a problem hiding this comment.
I discovered that using when to compare directly against some constants would allow the Kotlin compiler to generate a jump table, so I figured I'd just manually unroll the old loop based logic. Now the only branching in this method is the jump table for the when, compared to the chained if/else falling back to the loop in the previous implementation.
src/main/java/com/amazon/ion/impl/bin/IonRawBinaryWriter_1_1.kt
Outdated
Show resolved
Hide resolved
| // Just checking that data is written, not asserting the content. | ||
| assertTrue(actual.isNotBlank()) |
There was a problem hiding this comment.
Note: it would also be correct for the output to be blank in this particular case, since no user values were written. This is true for all the tests that write only an IVM before finish/close.
There was a problem hiding this comment.
Is that the behavior we want for Ion 1.1, though? What if someone calls writeIVM() and then finish() (maybe they have logic that writes after every top level piece of "data") and then try to write Ion 1.1 data? I know it's a niche case, but that would cause invalid data to be written.
In addition, because emitting an IVM resets the encoding environment, I think the decision of when to write or not write the IVM belongs in a higher level writer that is aware of the encoding environment.
There was a problem hiding this comment.
writeIVM() should be a raw-level API, not accessible to users of an application-level writer. Those users get IVMs written as a side effect of writing user values. The IVM that is written depends on the kind of writer the user selected (Ion 1.0 or Ion 1.1).
So, I think I'm saying I agree that for the purpose of these raw tests, the assertions of IVM writing are fine. The raw writer should always write an IVM when requested, but the user-level writer should only request one before writing a new encoding environment that precedes user values.
2 participants
Issue #, if available:
None.
Description of changes:
IonWriter_1_1with support for nulls, bools, and lists.WriteBuffertoFlexInt.WriteBufferso thatFlexIntandFlexUIntare written directly to the data array of the block instead of being indirected throughwriteByte().PatchPointto be a top level class so that it can be shared between writers.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.