Skip to content

Commit

Permalink
Try to isolate the problem
Browse files Browse the repository at this point in the history
  • Loading branch information
@popematt
popematt committed Aug 12, 2024
1 parent 661abe4 commit f32e59f
Show file tree
Hide file tree
Showing 3 changed files with 173 additions and 95 deletions.
194 changes: 102 additions & 92 deletions src/main/kotlin/com/amazon/ionelement/impl/IonElementLoaderImpl.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import com.amazon.ion.system.IonReaderBuilder
import com.amazon.ionelement.api.*
import com.amazon.ionelement.impl.collections.*
import java.util.ArrayDeque
import java.util.ArrayList
import kotlinx.collections.immutable.adapters.ImmutableListAdapter

internal class IonElementLoaderImpl(private val options: IonElementLoaderOptions) : IonElementLoader {
Expand Down Expand Up @@ -75,12 +76,105 @@ internal class IonElementLoaderImpl(private val options: IonElementLoaderOptions
}

override fun loadAllElements(ionReader: IonReader): List<AnyElement> {
val elements = mutableListOf<AnyElement>()
_loadAllElements(ionReader, elements as MutableList<Any>)
return elements.toImmutableListUnsafe()
return handleReaderException(ionReader) {
val elements = mutableListOf<AnyElement>()
while (ionReader.next() != null) {
val depth = ionReader.depth
elements.add(loadCurrentElement(ionReader))
check(depth == ionReader.depth)
}
elements
}
}

fun _loadAllElements(ionReader: IonReader, into: MutableList<Any>) {
override fun loadAllElements(ionText: String): List<AnyElement> =
IonReaderBuilder.standard().build(ionText).use(::loadAllElements)

override fun loadCurrentElement(ionReader: IonReader): AnyElement {
return private_loadCurrentElement(ionReader)
}

private fun private_loadCurrentElement(ionReader: IonReader): AnyElement {
return handleReaderException(ionReader) {
val valueType = requireNotNull(ionReader.type) { "The IonReader was not positioned at an element." }

// Read a value
val annotations = ionReader.typeAnnotations!!.toImmutableListUnsafe()

var metas = EMPTY_METAS
if (options.includeLocationMeta) {
val location = ionReader.currentLocation()
if (location != null) metas = location.toMetaContainer()
}

val element = if (ionReader.isNullValue) {
ionNull(valueType.toElementType(), annotations, metas).asAnyElement()
} else when (valueType) {
IonType.BOOL -> BoolElementImpl(ionReader.booleanValue(), annotations, metas)
IonType.INT -> when (ionReader.integerSize!!) {
IntegerSize.BIG_INTEGER -> {
val bigIntValue = ionReader.bigIntegerValue()
// Ion java's IonReader appears to determine integerSize based on number of bits,
// not on the actual value, which means if we have a padded int that is > 63 bits,
// but whose value only uses <= 63 bits then integerSize is still BIG_INTEGER.
// Compensate for that here...
if (bigIntValue !in RANGE_OF_LONG)
BigIntIntElementImpl(bigIntValue, annotations, metas)
else {
LongIntElementImpl(ionReader.longValue(), annotations, metas)
}
}

IntegerSize.LONG,
IntegerSize.INT -> LongIntElementImpl(ionReader.longValue(), annotations, metas)
}

IonType.FLOAT -> FloatElementImpl(ionReader.doubleValue(), annotations, metas)
IonType.DECIMAL -> DecimalElementImpl(ionReader.decimalValue(), annotations, metas)
IonType.TIMESTAMP -> TimestampElementImpl(ionReader.timestampValue(), annotations, metas)
IonType.STRING -> StringElementImpl(ionReader.stringValue(), annotations, metas)
IonType.SYMBOL -> SymbolElementImpl(ionReader.stringValue(), annotations, metas)
IonType.CLOB -> ClobElementImpl(ionReader.newBytes(), annotations, metas)
IonType.BLOB -> BlobElementImpl(ionReader.newBytes(), annotations, metas)
IonType.LIST -> {
val listContent = mutableListOf<AnyElement>()
val depth = ionReader.depth
ionReader.stepIn()
private_loadAllElements(ionReader, listContent as MutableList<Any>)
ionReader.stepOut()
check(depth == ionReader.depth)
ListElementImpl(listContent.toImmutableListUnsafe(), annotations, metas)
}

IonType.SEXP -> {
val sexpContent = mutableListOf<AnyElement>()
val depth = ionReader.depth
ionReader.stepIn()
private_loadAllElements(ionReader, sexpContent as MutableList<Any>)
ionReader.stepOut()
check(depth == ionReader.depth)
SexpElementImpl(sexpContent.toImmutableListUnsafe(), annotations, metas)
}

IonType.STRUCT -> {
val structContent = mutableListOf<StructField>()
val depth = ionReader.depth
ionReader.stepIn()
private_loadAllElements(ionReader, structContent as MutableList<Any>)
ionReader.stepOut()
check(depth == ionReader.depth)
StructElementImpl(structContent.toImmutableListUnsafe(), annotations, metas)
}

IonType.DATAGRAM -> error("IonElementLoaderImpl does not know what to do with IonType.DATAGRAM")
IonType.NULL -> error("IonType.NULL branch should be unreachable")

}.asAnyElement()
element
}
}

private fun private_loadAllElements(ionReader: IonReader, into: MutableList<Any>) {
// Intentionally not a recycling stack because we have mutable references that we are going to wrap as an
// ImmutableList and then forget about the reference to the mutable list.
val sequenceContentStack = ArrayDeque<MutableList<AnyElement>>()
Expand All @@ -94,7 +188,7 @@ internal class IonElementLoaderImpl(private val options: IonElementLoaderOptions

// End of container or input
if (valueType == null) {
if (sequenceContentStack.isEmpty()) {
if (sequenceContentStack.isEmpty() && fieldsStack.isEmpty()) {
return
} else {
ionReader.stepOut()
Expand Down Expand Up @@ -147,19 +241,19 @@ internal class IonElementLoaderImpl(private val options: IonElementLoaderOptions
IonType.CLOB -> ClobElementImpl(ionReader.newBytes(), annotations, metas)
IonType.BLOB -> BlobElementImpl(ionReader.newBytes(), annotations, metas)
IonType.LIST -> {
val listContent = mutableListOf<AnyElement>()
val listContent = ArrayList<AnyElement>()
nextElements = listContent as MutableList<Any>
ListElementImpl(ImmutableListAdapter(listContent), annotations, metas)
}

IonType.SEXP -> {
val sexpContent = mutableListOf<AnyElement>()
val sexpContent = ArrayList<AnyElement>()
nextElements = sexpContent as MutableList<Any>
SexpElementImpl(ImmutableListAdapter(sexpContent), annotations, metas)
}

IonType.STRUCT -> {
val structContent = mutableListOf<StructField>()
val structContent = ArrayList<StructField>()
nextElements = structContent as MutableList<Any>
StructElementImpl(ImmutableListAdapter(structContent), annotations, metas)
}
Expand Down Expand Up @@ -195,88 +289,4 @@ internal class IonElementLoaderImpl(private val options: IonElementLoaderOptions
)
}
}

fun _loadCurrentElement(ionReader: IonReader): AnyElement {
// Intentionally not a recycling stack because we have mutable references that we are going to wrap as an
// ImmutableList and then forget about the reference to the mutable list.

return handleReaderException(ionReader) {
val valueType = requireNotNull(ionReader.type) { "The IonReader was not positioned at an element." }

// Read a value
val annotations = ionReader.typeAnnotations!!.toImmutableListUnsafe()

var metas = EMPTY_METAS
if (options.includeLocationMeta) {
val location = ionReader.currentLocation()
if (location != null) metas = location.toMetaContainer()
}

val element = if (ionReader.isNullValue) {
ionNull(valueType.toElementType(), annotations, metas).asAnyElement()
} else when (valueType) {
IonType.BOOL -> BoolElementImpl(ionReader.booleanValue(), annotations, metas)
IonType.INT -> when (ionReader.integerSize!!) {
IntegerSize.BIG_INTEGER -> {
val bigIntValue = ionReader.bigIntegerValue()
// Ion java's IonReader appears to determine integerSize based on number of bits,
// not on the actual value, which means if we have a padded int that is > 63 bits,
// but whose value only uses <= 63 bits then integerSize is still BIG_INTEGER.
// Compensate for that here...
if (bigIntValue !in RANGE_OF_LONG)
BigIntIntElementImpl(bigIntValue, annotations, metas)
else {
LongIntElementImpl(ionReader.longValue(), annotations, metas)
}
}

IntegerSize.LONG,
IntegerSize.INT -> LongIntElementImpl(ionReader.longValue(), annotations, metas)
}

IonType.FLOAT -> FloatElementImpl(ionReader.doubleValue(), annotations, metas)
IonType.DECIMAL -> DecimalElementImpl(ionReader.decimalValue(), annotations, metas)
IonType.TIMESTAMP -> TimestampElementImpl(ionReader.timestampValue(), annotations, metas)
IonType.STRING -> StringElementImpl(ionReader.stringValue(), annotations, metas)
IonType.SYMBOL -> SymbolElementImpl(ionReader.stringValue(), annotations, metas)
IonType.CLOB -> ClobElementImpl(ionReader.newBytes(), annotations, metas)
IonType.BLOB -> BlobElementImpl(ionReader.newBytes(), annotations, metas)
IonType.LIST -> {
val listContent = mutableListOf<AnyElement>()
ionReader.stepIn()
_loadAllElements(ionReader, listContent as MutableList<Any>)
ionReader.stepOut()
ListElementImpl(listContent.toImmutableListUnsafe(), annotations, metas)
}

IonType.SEXP -> {
val sexpContent = mutableListOf<AnyElement>()
ionReader.stepIn()
_loadAllElements(ionReader, sexpContent as MutableList<Any>)
ionReader.stepOut()
SexpElementImpl(sexpContent.toImmutableListUnsafe(), annotations, metas)
}

IonType.STRUCT -> {
val structContent = mutableListOf<StructField>()
ionReader.stepIn()
_loadAllElements(ionReader, structContent as MutableList<Any>)
ionReader.stepOut()
StructElementImpl(structContent.toImmutableListUnsafe(), annotations, metas)
}

IonType.DATAGRAM -> error("IonElementLoaderImpl does not know what to do with IonType.DATAGRAM")
IonType.NULL -> error("IonType.NULL branch should be unreachable")

}.asAnyElement()
element
}
}

override fun loadAllElements(ionText: String): List<AnyElement> =
IonReaderBuilder.standard().build(ionText).use(::loadAllElements)

override fun loadCurrentElement(ionReader: IonReader): AnyElement {
return _loadCurrentElement(ionReader)
}
}
2 changes: 1 addition & 1 deletion src/main/kotlin/com/amazon/ionelement/impl/StructElementImpl.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal class StructElementImpl(
) : AnyElementBase(), StructElement {

override val type: ElementType get() = ElementType.STRUCT
override val size = allFields.size
override val size: Int get() = allFields.size

// Note that we are not using `by lazy` here because it requires 2 additional allocations and
// has been demonstrated to significantly increase memory consumption!
Expand Down
72 changes: 70 additions & 2 deletions src/test/kotlin/com/amazon/ionelement/IonElementLoaderTests.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import com.amazon.ionelement.util.convertToString
import kotlinx.collections.immutable.adapters.ImmutableListAdapter
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Disabled
import org.junit.jupiter.api.Test
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource
Expand Down Expand Up @@ -100,7 +101,11 @@ class IonElementLoaderTests {
IonElementLoaderTestCase("{foo:{bar:1}}", ionStructOf("foo" to ionStructOf("bar" to ionInt(1)))),
IonElementLoaderTestCase("{foo:[{}]}", ionStructOf("foo" to ionListOf(ionStructOf(emptyList())))),
IonElementLoaderTestCase("[{}]", ionListOf(ionStructOf(emptyList()))),
IonElementLoaderTestCase("[{}, {}]", ionListOf(ionStructOf(emptyList()), ionStructOf(emptyList()))),
IonElementLoaderTestCase("[{foo:1, bar: 2}]", ionListOf(ionStructOf("foo" to ionInt(1), "bar" to ionInt(2)))),
IonElementLoaderTestCase("{foo:[{bar:[{}]}]}",
ionStructOf("foo" to ionListOf(ionStructOf("bar" to ionListOf(ionStructOf(emptyList())))))),


)
}
Expand All @@ -124,6 +129,27 @@ class IonElementLoaderTests {

@Test
fun parsedAndConstructedInstances() {
assertEquals(
loadSingleElement("[1, a, true]"),
ionListOf(ionInt(1), ionSymbol("a"), ionBool(true))
)
assertEquals(
loadSingleElement("{foo:1}"),
ionStructOf("foo" to ionInt(1))
)


val struct1 = loadSingleElement("{foo:1}")
// Passes
assertEquals(
ionListOf(struct1),
ionListOf(ionStructOf("foo" to ionInt(1)))
)
// Fails
assertEquals(
ionListOf(struct1),
loadSingleElement("[{foo:1}]"),
)
assertEquals(
loadSingleElement("[{foo:1}]"),
ionListOf(ionStructOf("foo" to ionInt(1)))
Expand All @@ -138,6 +164,50 @@ class IonElementLoaderTests {
)
}

@Test
fun puzzling() {
val parsedList = loadSingleElement("[foo, 1]").asList()
val constructedList = ionListOf(ionSymbol("foo"), ionInt(1))
val parsedStruct = loadSingleElement("{foo:1}").asStruct()
val constructedStruct = ionStructOf("foo" to ionInt(1))
val parsedListOfStruct = loadSingleElement("[{foo:1}]").asList()
val constructedListOfStruct = ionListOf(ionStructOf("foo" to ionInt(1)))

println("Parsed vs Constructed")
// TRUE
println(parsedList == constructedList)
println(constructedList == parsedList)
println(parsedStruct == constructedStruct)
println(constructedStruct == parsedStruct)
// FALSE
println(parsedListOfStruct == constructedListOfStruct)
println(constructedListOfStruct == parsedListOfStruct)

println("Mix of parsed and constructed")

// True
println(constructedListOfStruct == ionListOf(parsedStruct))
println(constructedListOfStruct == ionListOf(constructedStruct))

// False
println(parsedListOfStruct == ionListOf(parsedStruct))
println(parsedListOfStruct == ionListOf(constructedStruct))
println(ionListOf(parsedStruct) == parsedListOfStruct)
println(ionListOf(constructedStruct) == parsedListOfStruct)

// True
println(ionListOf(parsedStruct) == ionListOf(constructedStruct))

// True
println(parsedListOfStruct == loadSingleElement(" [{foo:1}]"))

println("As Strings")
// true
println("$constructedList" == "$parsedList")
println("$constructedStruct" == "$parsedStruct")
println("$constructedListOfStruct" == "$parsedListOfStruct")
}

@Test
fun kotlinIdiomaticTest2() {

Expand Down Expand Up @@ -174,7 +244,5 @@ class IonElementLoaderTests {
parsedIonElement.asAnyElement().asList().values[0].asStruct()
)
// assertEquals(tc.expectedElement, parsedIonElement)

// assertEquals(tc.expectedElement, parsedIonElement)
}
}

0 comments on commit f32e59f

Please sign in to comment.