Skip to content

Commit

Permalink
Ensure that all empty PersistentLists are the same instance.
Browse files Browse the repository at this point in the history
  • Loading branch information
popematt committed Mar 14, 2024
1 parent 5b9ec91 commit df159ac
Show file tree
Hide file tree
Showing 15 changed files with 74 additions and 49 deletions.
77 changes: 58 additions & 19 deletions src/com/amazon/ionelement/api/Ion.kt
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import com.amazon.ionelement.impl.SymbolElementImpl
import com.amazon.ionelement.impl.TimestampElementImpl
import java.math.BigInteger
import kotlinx.collections.immutable.PersistentList
import kotlinx.collections.immutable.persistentListOf
import kotlinx.collections.immutable.toPersistentList
import kotlinx.collections.immutable.toPersistentMap

Expand Down Expand Up @@ -124,7 +125,7 @@ public fun ionString(
metas: MetaContainer = emptyMetaContainer()
): StringElement = StringElementImpl(
value = s,
annotations = annotations.toPersistentList(),
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
)
/** Creates a [StringElement] that represents an Ion `symbol`. */
Expand All @@ -141,7 +142,7 @@ public fun ionSymbol(
metas: MetaContainer = emptyMetaContainer()
): SymbolElement = SymbolElementImpl(
value = s,
annotations = annotations.toPersistentList(),
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
)

Expand All @@ -159,7 +160,7 @@ public fun ionTimestamp(
metas: MetaContainer = emptyMetaContainer()
): TimestampElement = TimestampElementImpl(
timestampValue = Timestamp.valueOf(s),
annotations = annotations.toPersistentList(),
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
)

Expand All @@ -177,7 +178,7 @@ public fun ionTimestamp(
metas: MetaContainer = emptyMetaContainer()
): TimestampElement = TimestampElementImpl(
timestampValue = timestamp,
annotations = annotations.toPersistentList(),
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
)

Expand All @@ -196,7 +197,7 @@ public fun ionInt(
): IntElement =
LongIntElementImpl(
longValue = l,
annotations = annotations.toPersistentList(),
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
)

Expand All @@ -214,7 +215,7 @@ public fun ionInt(
metas: MetaContainer = emptyMetaContainer()
): IntElement = BigIntIntElementImpl(
bigIntegerValue = bigInt,
annotations = annotations.toPersistentList(),
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
)

Expand All @@ -232,7 +233,7 @@ public fun ionBool(
metas: MetaContainer = emptyMetaContainer()
): BoolElement = BoolElementImpl(
booleanValue = b,
annotations = annotations.toPersistentList(),
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
)

Expand All @@ -250,7 +251,7 @@ public fun ionFloat(
metas: MetaContainer = emptyMetaContainer()
): FloatElement = FloatElementImpl(
doubleValue = d,
annotations = annotations.toPersistentList(),
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
)

Expand All @@ -268,7 +269,7 @@ public fun ionDecimal(
metas: MetaContainer = emptyMetaContainer()
): DecimalElement = DecimalElementImpl(
decimalValue = bigDecimal,
annotations = annotations.toPersistentList(),
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
)

Expand All @@ -290,7 +291,7 @@ public fun ionBlob(
metas: MetaContainer = emptyMetaContainer()
): BlobElement = BlobElementImpl(
bytes = bytes.clone(),
annotations = annotations.toPersistentList(),
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
)

Expand Down Expand Up @@ -319,7 +320,7 @@ public fun ionClob(
metas: MetaContainer = emptyMetaContainer()
): ClobElement = ClobElementImpl(
bytes = bytes.clone(),
annotations = annotations.toPersistentList(),
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
)
/**
Expand All @@ -343,8 +344,8 @@ public fun ionListOf(
metas: MetaContainer = emptyMetaContainer()
): ListElement =
ListElementImpl(
values = iterable.map { it.asAnyElement() }.toPersistentList(),
annotations = annotations.toPersistentList(),
values = iterable.mapToEmptyOrPersistentList { it.asAnyElement() },
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
)

Expand Down Expand Up @@ -392,8 +393,8 @@ public fun ionSexpOf(
metas: MetaContainer = emptyMetaContainer()
): SexpElement =
SexpElementImpl(
values = iterable.map { it.asAnyElement() }.toPersistentList(),
annotations = annotations.toPersistentList(),
values = iterable.mapToEmptyOrPersistentList { it.asAnyElement() },
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
)

Expand Down Expand Up @@ -434,8 +435,8 @@ public fun ionStructOf(
metas: MetaContainer = emptyMetaContainer()
): StructElement =
StructElementImpl(
allFields = fields.toPersistentList(),
annotations = annotations.toPersistentList(),
allFields = fields.toEmptyOrPersistentList(),
annotations = annotations.toEmptyOrPersistentList(),
metas = metas.toPersistentMap()
)

Expand All @@ -454,7 +455,7 @@ public fun ionStructOf(
): StructElement =
ionStructOf(
fields = fields.asIterable(),
annotations = annotations.toPersistentList(),
annotations = annotations.toEmptyOrPersistentList(),
metas = metas
)

Expand Down Expand Up @@ -482,8 +483,46 @@ 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
private val EMPTY_PERSISTENT_LIST: PersistentList<Nothing> = emptyList<Nothing>().toPersistentList()
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)
Expand Down
3 changes: 1 addition & 2 deletions src/com/amazon/ionelement/impl/BigIntIntElementImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import com.amazon.ionelement.api.PersistentMetaContainer
import com.amazon.ionelement.api.constraintError
import java.math.BigInteger
import kotlinx.collections.immutable.PersistentList
import kotlinx.collections.immutable.toPersistentList
import kotlinx.collections.immutable.toPersistentMap

internal class BigIntIntElementImpl(
Expand All @@ -42,7 +41,7 @@ internal class BigIntIntElementImpl(
}

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

override fun withAnnotations(vararg additionalAnnotations: String): BigIntIntElementImpl = _withAnnotations(*additionalAnnotations)
override fun withAnnotations(additionalAnnotations: Iterable<String>): BigIntIntElementImpl = _withAnnotations(additionalAnnotations)
Expand Down
3 changes: 1 addition & 2 deletions src/com/amazon/ionelement/impl/BlobElementImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ 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.toPersistentList
import kotlinx.collections.immutable.toPersistentMap

internal class BlobElementImpl(
Expand All @@ -33,7 +32,7 @@ internal class BlobElementImpl(
override fun writeContentTo(writer: IonWriter) = writer.writeBlob(bytes)

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

override fun withAnnotations(vararg additionalAnnotations: String): BlobElementImpl = _withAnnotations(*additionalAnnotations)
override fun withAnnotations(additionalAnnotations: Iterable<String>): BlobElementImpl = _withAnnotations(additionalAnnotations)
Expand Down
3 changes: 1 addition & 2 deletions src/com/amazon/ionelement/impl/BoolElementImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ 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.toPersistentList
import kotlinx.collections.immutable.toPersistentMap

internal class BoolElementImpl(
Expand All @@ -30,7 +29,7 @@ internal class BoolElementImpl(
override val type: ElementType get() = ElementType.BOOL

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

override fun withAnnotations(vararg additionalAnnotations: String): BoolElementImpl = _withAnnotations(*additionalAnnotations)
override fun withAnnotations(additionalAnnotations: Iterable<String>): BoolElementImpl = _withAnnotations(additionalAnnotations)
Expand Down
3 changes: 1 addition & 2 deletions src/com/amazon/ionelement/impl/ClobElementImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ 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.toPersistentList
import kotlinx.collections.immutable.toPersistentMap

internal class ClobElementImpl(
Expand All @@ -32,7 +31,7 @@ internal class ClobElementImpl(

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

override fun withAnnotations(vararg additionalAnnotations: String): ClobElementImpl = _withAnnotations(*additionalAnnotations)
override fun withAnnotations(additionalAnnotations: Iterable<String>): ClobElementImpl = _withAnnotations(additionalAnnotations)
Expand Down
3 changes: 1 addition & 2 deletions src/com/amazon/ionelement/impl/DecimalElementImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ 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.toPersistentList
import kotlinx.collections.immutable.toPersistentMap

internal class DecimalElementImpl(
Expand All @@ -31,7 +30,7 @@ internal class DecimalElementImpl(
override val type get() = ElementType.DECIMAL

override fun copy(annotations: List<String>, metas: MetaContainer): DecimalElementImpl =
DecimalElementImpl(decimalValue, annotations.toPersistentList(), metas.toPersistentMap())
DecimalElementImpl(decimalValue, annotations.toEmptyOrPersistentList(), metas.toPersistentMap())

override fun withAnnotations(vararg additionalAnnotations: String): DecimalElementImpl = _withAnnotations(*additionalAnnotations)
override fun withAnnotations(additionalAnnotations: Iterable<String>): DecimalElementImpl = _withAnnotations(additionalAnnotations)
Expand Down
3 changes: 1 addition & 2 deletions src/com/amazon/ionelement/impl/FloatElementImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ 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.toPersistentList
import kotlinx.collections.immutable.toPersistentMap

internal class FloatElementImpl(
Expand All @@ -30,7 +29,7 @@ internal class FloatElementImpl(
override val type: ElementType get() = ElementType.FLOAT

override fun copy(annotations: List<String>, metas: MetaContainer): FloatElementImpl =
FloatElementImpl(doubleValue, annotations.toPersistentList(), metas.toPersistentMap())
FloatElementImpl(doubleValue, annotations.toEmptyOrPersistentList(), metas.toPersistentMap())

override fun withAnnotations(vararg additionalAnnotations: String): FloatElementImpl = _withAnnotations(*additionalAnnotations)
override fun withAnnotations(additionalAnnotations: Iterable<String>): FloatElementImpl = _withAnnotations(additionalAnnotations)
Expand Down
3 changes: 1 addition & 2 deletions src/com/amazon/ionelement/impl/ListElementImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package com.amazon.ionelement.impl
import com.amazon.ionelement.api.*
import com.amazon.ionelement.api.PersistentMetaContainer
import kotlinx.collections.immutable.PersistentList
import kotlinx.collections.immutable.toPersistentList
import kotlinx.collections.immutable.toPersistentMap

internal class ListElementImpl(
Expand All @@ -31,7 +30,7 @@ internal class ListElementImpl(
override val listValues: List<AnyElement> get() = values

override fun copy(annotations: List<String>, metas: MetaContainer): ListElementImpl =
ListElementImpl(values, annotations.toPersistentList(), metas.toPersistentMap())
ListElementImpl(values, annotations.toEmptyOrPersistentList(), metas.toPersistentMap())

override fun withAnnotations(vararg additionalAnnotations: String): ListElementImpl = _withAnnotations(*additionalAnnotations)
override fun withAnnotations(additionalAnnotations: Iterable<String>): ListElementImpl = _withAnnotations(additionalAnnotations)
Expand Down
3 changes: 1 addition & 2 deletions src/com/amazon/ionelement/impl/LongIntElementImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import com.amazon.ionelement.api.*
import com.amazon.ionelement.api.PersistentMetaContainer
import java.math.BigInteger
import kotlinx.collections.immutable.PersistentList
import kotlinx.collections.immutable.toPersistentList
import kotlinx.collections.immutable.toPersistentMap

internal class LongIntElementImpl(
Expand All @@ -34,7 +33,7 @@ internal class LongIntElementImpl(
override val bigIntegerValue: BigInteger get() = BigInteger.valueOf(longValue)

override fun copy(annotations: List<String>, metas: MetaContainer): LongIntElementImpl =
LongIntElementImpl(longValue, annotations.toPersistentList(), metas.toPersistentMap())
LongIntElementImpl(longValue, annotations.toEmptyOrPersistentList(), metas.toPersistentMap())

override fun withAnnotations(vararg additionalAnnotations: String): LongIntElementImpl = _withAnnotations(*additionalAnnotations)
override fun withAnnotations(additionalAnnotations: Iterable<String>): LongIntElementImpl = _withAnnotations(additionalAnnotations)
Expand Down
3 changes: 1 addition & 2 deletions src/com/amazon/ionelement/impl/NullElementImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ 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.toPersistentList
import kotlinx.collections.immutable.toPersistentMap

internal class NullElementImpl(
Expand All @@ -31,7 +30,7 @@ internal class NullElementImpl(
override val isNull: Boolean get() = true

override fun copy(annotations: List<String>, metas: MetaContainer): AnyElement =
NullElementImpl(type, annotations.toPersistentList(), metas.toPersistentMap())
NullElementImpl(type, annotations.toEmptyOrPersistentList(), metas.toPersistentMap())

override fun withAnnotations(vararg additionalAnnotations: String): AnyElement = _withAnnotations(*additionalAnnotations)
override fun withAnnotations(additionalAnnotations: Iterable<String>): AnyElement = _withAnnotations(additionalAnnotations)
Expand Down
3 changes: 1 addition & 2 deletions src/com/amazon/ionelement/impl/SexpElementImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package com.amazon.ionelement.impl
import com.amazon.ionelement.api.*
import com.amazon.ionelement.api.PersistentMetaContainer
import kotlinx.collections.immutable.PersistentList
import kotlinx.collections.immutable.toPersistentList
import kotlinx.collections.immutable.toPersistentMap

internal class SexpElementImpl(
Expand All @@ -31,7 +30,7 @@ internal class SexpElementImpl(
override val sexpValues: List<AnyElement> get() = seqValues

override fun copy(annotations: List<String>, metas: MetaContainer): SexpElementImpl =
SexpElementImpl(values, annotations.toPersistentList(), metas.toPersistentMap())
SexpElementImpl(values, annotations.toEmptyOrPersistentList(), metas.toPersistentMap())

override fun withAnnotations(vararg additionalAnnotations: String): SexpElementImpl = _withAnnotations(*additionalAnnotations)
override fun withAnnotations(additionalAnnotations: Iterable<String>): SexpElementImpl = _withAnnotations(additionalAnnotations)
Expand Down
3 changes: 1 addition & 2 deletions src/com/amazon/ionelement/impl/StringElementImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ 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.toPersistentList
import kotlinx.collections.immutable.toPersistentMap

internal class StringElementImpl(
Expand All @@ -32,7 +31,7 @@ internal class StringElementImpl(
override val stringValue: String get() = textValue

override fun copy(annotations: List<String>, metas: MetaContainer): StringElementImpl =
StringElementImpl(textValue, annotations.toPersistentList(), metas.toPersistentMap())
StringElementImpl(textValue, annotations.toEmptyOrPersistentList(), metas.toPersistentMap())

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

0 comments on commit df159ac

Please sign in to comment.