Skip to content

Commit

Permalink
Replaces persistent collections with immutable collections (#101)
Browse files Browse the repository at this point in the history
* Replaces kotlinx.collections.immutable dependency with custom immutable collections

* Clean up unused code

* Use Immutable collection adapters from kotlinx.collections.immutables

* Add build.gradle.kts changes forgotten from last commit
  • Loading branch information
popematt authored Aug 5, 2024
1 parent eb6636f commit b635d5c
Show file tree
Hide file tree
Showing 24 changed files with 483 additions and 201 deletions.
2 changes: 1 addition & 1 deletion build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ val isReleaseVersion: Boolean = !version.toString().endsWith("SNAPSHOT")
dependencies {
api("com.amazon.ion:ion-java:[1.4.0,)")
compileOnly("com.amazon.ion:ion-java:1.4.0")
implementation("org.jetbrains.kotlin:kotlin-stdlib-jdk8")
implementation("org.jetbrains.kotlinx:kotlinx-collections-immutable:0.3.4")
implementation("org.jetbrains.kotlin:kotlin-stdlib-jdk8")
testImplementation("org.jetbrains.kotlin:kotlin-test")
testImplementation("org.junit.jupiter:junit-jupiter-api:5.6.2")
testImplementation("org.junit.jupiter:junit-jupiter-params:5.6.2")
Expand Down
123 changes: 39 additions & 84 deletions src/main/kotlin/com/amazon/ionelement/api/Ion.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package com.amazon.ionelement.api

import com.amazon.ion.Decimal
import com.amazon.ion.Timestamp
import com.amazon.ionelement.impl.*
import com.amazon.ionelement.impl.BigIntIntElementImpl
import com.amazon.ionelement.impl.BlobElementImpl
import com.amazon.ionelement.impl.BoolElementImpl
Expand All @@ -34,12 +33,9 @@ import com.amazon.ionelement.impl.StructElementImpl
import com.amazon.ionelement.impl.StructFieldImpl
import com.amazon.ionelement.impl.SymbolElementImpl
import com.amazon.ionelement.impl.TimestampElementImpl
import com.amazon.ionelement.impl.collections.*
import java.math.BigInteger
import java.util.function.Consumer
import kotlinx.collections.immutable.PersistentList
import kotlinx.collections.immutable.persistentListOf
import kotlinx.collections.immutable.toPersistentList
import kotlinx.collections.immutable.toPersistentMap

internal typealias Annotations = List<String>

Expand Down Expand Up @@ -127,8 +123,8 @@ public fun ionString(
metas: MetaContainer = emptyMetaContainer()
): StringElement = StringElementImpl(
value = s,
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
annotations = annotations.toImmutableList(),
metas = metas.toImmutableMap()
)
/** Creates a [StringElement] that represents an Ion `symbol`. */
public fun ionString(
Expand All @@ -144,8 +140,8 @@ public fun ionSymbol(
metas: MetaContainer = emptyMetaContainer()
): SymbolElement = SymbolElementImpl(
value = s,
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
annotations = annotations.toImmutableList(),
metas = metas.toImmutableMap()
)

/** Creates a [SymbolElement] that represents an Ion `symbol`. */
Expand All @@ -162,8 +158,8 @@ public fun ionTimestamp(
metas: MetaContainer = emptyMetaContainer()
): TimestampElement = TimestampElementImpl(
timestampValue = Timestamp.valueOf(s),
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
annotations = annotations.toImmutableList(),
metas = metas.toImmutableMap()
)

/** Creates a [TimestampElement] that represents an Ion `timestamp`. */
Expand All @@ -180,8 +176,8 @@ public fun ionTimestamp(
metas: MetaContainer = emptyMetaContainer()
): TimestampElement = TimestampElementImpl(
timestampValue = timestamp,
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
annotations = annotations.toImmutableList(),
metas = metas.toImmutableMap()
)

/** Creates a [TimestampElement] that represents an Ion `timestamp`. */
Expand All @@ -199,8 +195,8 @@ public fun ionInt(
): IntElement =
LongIntElementImpl(
longValue = l,
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
annotations = annotations.toImmutableList(),
metas = metas.toImmutableMap()
)

/** Creates an [IntElement] that represents an Ion `int`. */
Expand All @@ -217,8 +213,8 @@ public fun ionInt(
metas: MetaContainer = emptyMetaContainer()
): IntElement = BigIntIntElementImpl(
bigIntegerValue = bigInt,
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
annotations = annotations.toImmutableList(),
metas = metas.toImmutableMap()
)

/** Creates an [IntElement] that represents an Ion `BitInteger`. */
Expand All @@ -235,8 +231,8 @@ public fun ionBool(
metas: MetaContainer = emptyMetaContainer()
): BoolElement = BoolElementImpl(
booleanValue = b,
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
annotations = annotations.toImmutableList(),
metas = metas.toImmutableMap()
)

/** Creates a [BoolElement] that represents an Ion `bool`. */
Expand All @@ -253,8 +249,8 @@ public fun ionFloat(
metas: MetaContainer = emptyMetaContainer()
): FloatElement = FloatElementImpl(
doubleValue = d,
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
annotations = annotations.toImmutableList(),
metas = metas.toImmutableMap()
)

/** Creates a [FloatElement] that represents an Ion `float`. */
Expand All @@ -271,8 +267,8 @@ public fun ionDecimal(
metas: MetaContainer = emptyMetaContainer()
): DecimalElement = DecimalElementImpl(
decimalValue = bigDecimal,
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
annotations = annotations.toImmutableList(),
metas = metas.toImmutableMap()
)

/** Creates a [DecimalElement] that represents an Ion `decimall`. */
Expand All @@ -293,8 +289,8 @@ public fun ionBlob(
metas: MetaContainer = emptyMetaContainer()
): BlobElement = BlobElementImpl(
bytes = bytes.clone(),
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
annotations = annotations.toImmutableList(),
metas = metas.toImmutableMap()
)

/**
Expand Down Expand Up @@ -322,8 +318,8 @@ public fun ionClob(
metas: MetaContainer = emptyMetaContainer()
): ClobElement = ClobElementImpl(
bytes = bytes.clone(),
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
annotations = annotations.toImmutableList(),
metas = metas.toImmutableMap()
)
/**
* Creates a [ClobElement] that represents an Ion `clob`.
Expand All @@ -346,9 +342,9 @@ public fun ionListOf(
metas: MetaContainer = emptyMetaContainer()
): ListElement =
ListElementImpl(
values = iterable.mapToEmptyOrPersistentList { it.asAnyElement() },
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
values = iterable.map { it.asAnyElement() }.toImmutableListUnsafe(),
annotations = annotations.toImmutableList(),
metas = metas.toImmutableMap()
)

/** Creates a [ListElement] that represents an Ion `list`. */
Expand Down Expand Up @@ -395,9 +391,9 @@ public fun ionSexpOf(
metas: MetaContainer = emptyMetaContainer()
): SexpElement =
SexpElementImpl(
values = iterable.mapToEmptyOrPersistentList { it.asAnyElement() },
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
values = iterable.map { it.asAnyElement() }.toImmutableListUnsafe(),
annotations = annotations.toImmutableList(),
metas = metas.toImmutableMap()
)

/** Creates an [SexpElement] that represents an Ion `sexp`. */
Expand Down Expand Up @@ -437,9 +433,9 @@ public fun ionStructOf(
metas: MetaContainer = emptyMetaContainer()
): StructElement =
StructElementImpl(
allFields = fields.toEmptyOrPersistentList(),
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
allFields = fields.toImmutableList(),
annotations = annotations.toImmutableList(),
metas = metas.toImmutableMap()
)

/** Creates a [StructElement] that represents an Ion `struct` with the specified fields. */
Expand All @@ -457,7 +453,7 @@ public fun ionStructOf(
): StructElement =
ionStructOf(
fields = fields.asIterable(),
annotations = annotations.toEmptyOrPersistentList(),
annotations = annotations.toImmutableList(),
metas = metas
)

Expand Down Expand Up @@ -523,55 +519,14 @@ public fun buildStruct(
return ionStructOf(fields, annotations, metas)
}

/**
* Converts an `Iterable` to a `PersistentList`.
*
* ### Why is this needed?
* `kotlinx.collections.immutable` <= 0.3.7 has a bug that causes it to unnecessarily allocate empty `PersistentList`s
* instead of using a singleton instance. The fix is in
* [kotlinx.collections.immutable#176](https://github.com/Kotlin/kotlinx.collections.immutable/pull/176),
* but we cannot use it because the version of `kotlinx.collections.immutable` that will have (or has) the fix
* requires Kotlin stdlib >=1.9.2, and `ion-element-kotlin` supports consumers using Kotlin >= 1.3.0.
*/
internal fun <E> Iterable<E>.toEmptyOrPersistentList(): PersistentList<E> {
val isEmpty = if (this is Collection<*>) {
this.isEmpty()
} else {
!this.iterator().hasNext()
}
return if (isEmpty) EMPTY_PERSISTENT_LIST else toPersistentList()
}

/**
* Converts an `Iterable` to a `PersistentList`, transforming each element using [block].
*
* ### Why is this needed?
* `kotlinx.collections.immutable` <= 0.3.7 has a bug that causes it to unnecessarily allocate empty `PersistentList`s
* instead of using a singleton instance. The fix is in
* [kotlinx.collections.immutable#176](https://github.com/Kotlin/kotlinx.collections.immutable/pull/176),
* but we cannot use it because the version of `kotlinx.collections.immutable` that will have (or has) the fix
* requires Kotlin stdlib >=1.9.2, and `ion-element-kotlin` supports consumers using Kotlin >= 1.3.0.
*/
internal inline fun <T, R> Iterable<T>.mapToEmptyOrPersistentList(block: (T) -> R): PersistentList<R> {
val isEmpty = if (this is Collection<*>) {
this.isEmpty()
} else {
!this.iterator().hasNext()
}
return if (isEmpty) EMPTY_PERSISTENT_LIST else mapTo(persistentListOf<R>().builder(), block).build()
}

// Memoized empty PersistentList for the memoized container types and null values
internal val EMPTY_PERSISTENT_LIST: PersistentList<Nothing> = persistentListOf()

// Memoized empty instances of our container types.
private val EMPTY_LIST = ListElementImpl(EMPTY_PERSISTENT_LIST, EMPTY_PERSISTENT_LIST, EMPTY_METAS)
private val EMPTY_SEXP = SexpElementImpl(EMPTY_PERSISTENT_LIST, EMPTY_PERSISTENT_LIST, EMPTY_METAS)
private val EMPTY_STRUCT = StructElementImpl(EMPTY_PERSISTENT_LIST, EMPTY_PERSISTENT_LIST, EMPTY_METAS)
private val EMPTY_BLOB = BlobElementImpl(ByteArray(0), EMPTY_PERSISTENT_LIST, EMPTY_METAS)
private val EMPTY_CLOB = ClobElementImpl(ByteArray(0), EMPTY_PERSISTENT_LIST, EMPTY_METAS)
private val EMPTY_LIST = ListElementImpl(EMPTY_IMMUTABLE_LIST, EMPTY_IMMUTABLE_LIST, EMPTY_METAS)
private val EMPTY_SEXP = SexpElementImpl(EMPTY_IMMUTABLE_LIST, EMPTY_IMMUTABLE_LIST, EMPTY_METAS)
private val EMPTY_STRUCT = StructElementImpl(EMPTY_IMMUTABLE_LIST, EMPTY_IMMUTABLE_LIST, EMPTY_METAS)
private val EMPTY_BLOB = BlobElementImpl(ByteArray(0), EMPTY_IMMUTABLE_LIST, EMPTY_METAS)
private val EMPTY_CLOB = ClobElementImpl(ByteArray(0), EMPTY_IMMUTABLE_LIST, EMPTY_METAS)

// Memoized instances of all of our null values.
private val ALL_NULLS = ElementType.values().map {
it to NullElementImpl(it, EMPTY_PERSISTENT_LIST, EMPTY_METAS) as IonElement
it to NullElementImpl(it, EMPTY_IMMUTABLE_LIST, EMPTY_METAS) as IonElement
}.toMap()
7 changes: 3 additions & 4 deletions src/main/kotlin/com/amazon/ionelement/api/IonMeta.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,12 @@
@file: JvmName("IonMeta")
package com.amazon.ionelement.api

import kotlinx.collections.immutable.PersistentMap
import kotlinx.collections.immutable.toPersistentHashMap
import com.amazon.ionelement.impl.collections.*

public typealias MetaContainer = Map<String, Any>
internal typealias PersistentMetaContainer = PersistentMap<String, Any>
internal typealias ImmutableMetaContainer = ImmutableMap<String, Any>

internal val EMPTY_METAS = HashMap<String, Any>().toPersistentHashMap()
internal val EMPTY_METAS: ImmutableMetaContainer = emptyMap<String, Any>().toImmutableMap()

public fun emptyMetaContainer(): MetaContainer = EMPTY_METAS

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,15 @@ package com.amazon.ionelement.impl

import com.amazon.ion.IonWriter
import com.amazon.ionelement.api.*
import com.amazon.ionelement.api.PersistentMetaContainer
import com.amazon.ionelement.api.ImmutableMetaContainer
import com.amazon.ionelement.api.constraintError
import com.amazon.ionelement.impl.collections.*
import java.math.BigInteger
import kotlinx.collections.immutable.PersistentList
import kotlinx.collections.immutable.toPersistentMap

internal class BigIntIntElementImpl(
override val bigIntegerValue: BigInteger,
override val annotations: PersistentList<String>,
override val metas: PersistentMetaContainer
override val annotations: ImmutableList<String>,
override val metas: ImmutableMetaContainer
) : AnyElementBase(), IntElement {

override val type: ElementType get() = ElementType.INT
Expand All @@ -42,7 +41,7 @@ internal class BigIntIntElementImpl(
}

override fun copy(annotations: List<String>, metas: MetaContainer): BigIntIntElementImpl =
BigIntIntElementImpl(bigIntegerValue, annotations.toEmptyOrPersistentList(), metas.toPersistentMap())
BigIntIntElementImpl(bigIntegerValue, annotations.toImmutableList(), metas.toImmutableMap())

override fun withAnnotations(vararg additionalAnnotations: String): BigIntIntElementImpl = _withAnnotations(*additionalAnnotations)
override fun withAnnotations(additionalAnnotations: Iterable<String>): BigIntIntElementImpl = _withAnnotations(additionalAnnotations)
Expand Down
11 changes: 5 additions & 6 deletions src/main/kotlin/com/amazon/ionelement/impl/BlobElementImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,13 @@ package com.amazon.ionelement.impl

import com.amazon.ion.IonWriter
import com.amazon.ionelement.api.*
import com.amazon.ionelement.api.PersistentMetaContainer
import kotlinx.collections.immutable.PersistentList
import kotlinx.collections.immutable.toPersistentMap
import com.amazon.ionelement.api.ImmutableMetaContainer
import com.amazon.ionelement.impl.collections.*

internal class BlobElementImpl(
bytes: ByteArray,
override val annotations: PersistentList<String>,
override val metas: PersistentMetaContainer
override val annotations: ImmutableList<String>,
override val metas: ImmutableMetaContainer
) : LobElementBase(bytes), BlobElement {

override val type: ElementType get() = ElementType.BLOB
Expand All @@ -34,7 +33,7 @@ internal class BlobElementImpl(
override fun writeContentTo(writer: IonWriter) = writer.writeBlob(bytes)

override fun copy(annotations: List<String>, metas: MetaContainer): BlobElementImpl =
BlobElementImpl(bytes, annotations.toEmptyOrPersistentList(), metas.toPersistentMap())
BlobElementImpl(bytes, annotations.toImmutableList(), metas.toImmutableMap())

override fun withAnnotations(vararg additionalAnnotations: String): BlobElementImpl = _withAnnotations(*additionalAnnotations)
override fun withAnnotations(additionalAnnotations: Iterable<String>): BlobElementImpl = _withAnnotations(additionalAnnotations)
Expand Down
11 changes: 5 additions & 6 deletions src/main/kotlin/com/amazon/ionelement/impl/BoolElementImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,18 @@ package com.amazon.ionelement.impl

import com.amazon.ion.IonWriter
import com.amazon.ionelement.api.*
import com.amazon.ionelement.api.PersistentMetaContainer
import kotlinx.collections.immutable.PersistentList
import kotlinx.collections.immutable.toPersistentMap
import com.amazon.ionelement.api.ImmutableMetaContainer
import com.amazon.ionelement.impl.collections.*

internal class BoolElementImpl(
override val booleanValue: Boolean,
override val annotations: PersistentList<String>,
override val metas: PersistentMetaContainer
override val annotations: ImmutableList<String>,
override val metas: ImmutableMetaContainer
) : AnyElementBase(), BoolElement {
override val type: ElementType get() = ElementType.BOOL

override fun copy(annotations: List<String>, metas: MetaContainer): BoolElementImpl =
BoolElementImpl(booleanValue, annotations.toEmptyOrPersistentList(), metas.toPersistentMap())
BoolElementImpl(booleanValue, annotations.toImmutableList(), metas.toImmutableMap())

override fun withAnnotations(vararg additionalAnnotations: String): BoolElementImpl = _withAnnotations(*additionalAnnotations)
override fun withAnnotations(additionalAnnotations: Iterable<String>): BoolElementImpl = _withAnnotations(additionalAnnotations)
Expand Down
11 changes: 5 additions & 6 deletions src/main/kotlin/com/amazon/ionelement/impl/ClobElementImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,13 @@ package com.amazon.ionelement.impl

import com.amazon.ion.IonWriter
import com.amazon.ionelement.api.*
import com.amazon.ionelement.api.PersistentMetaContainer
import kotlinx.collections.immutable.PersistentList
import kotlinx.collections.immutable.toPersistentMap
import com.amazon.ionelement.api.ImmutableMetaContainer
import com.amazon.ionelement.impl.collections.*

internal class ClobElementImpl(
bytes: ByteArray,
override val annotations: PersistentList<String>,
override val metas: PersistentMetaContainer
override val annotations: ImmutableList<String>,
override val metas: ImmutableMetaContainer
) : LobElementBase(bytes), ClobElement {

override val type: ElementType get() = ElementType.CLOB
Expand All @@ -33,7 +32,7 @@ internal class ClobElementImpl(

override fun writeContentTo(writer: IonWriter) = writer.writeClob(bytes)
override fun copy(annotations: List<String>, metas: MetaContainer): ClobElementImpl =
ClobElementImpl(bytes, annotations.toEmptyOrPersistentList(), metas.toPersistentMap())
ClobElementImpl(bytes, annotations.toImmutableList(), metas.toImmutableMap())

override fun withAnnotations(vararg additionalAnnotations: String): ClobElementImpl = _withAnnotations(*additionalAnnotations)
override fun withAnnotations(additionalAnnotations: Iterable<String>): ClobElementImpl = _withAnnotations(additionalAnnotations)
Expand Down
Loading

0 comments on commit b635d5c

Please sign in to comment.