From a9f5380ea41877cbf5ae892baa813755b503d26d Mon Sep 17 00:00:00 2001 From: Tyler Gregg Date: Tue, 16 Sep 2025 12:01:18 -0700 Subject: [PATCH 1/3] Resurrects the stream copy optimization feature that was dropped in v1.11.0. --- .../IonReaderContinuableTopLevelBinary.java | 14 +++ .../OptimizedBinaryWriterSymbolTableTest.java | 92 +++++++++++++++++++ .../impl/OptimizedBinaryWriterTestCase.java | 5 +- 3 files changed, 107 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java b/src/main/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java index e46118bc0f..92557cded8 100644 --- a/src/main/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java +++ b/src/main/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java @@ -17,6 +17,7 @@ import com.amazon.ion.SymbolTable; import com.amazon.ion.system.IonReaderBuilder; +import java.io.IOException; import java.io.InputStream; /** @@ -315,6 +316,14 @@ public void hoist(Span span) { } } + private class ByteTransferReaderFacet implements _Private_ByteTransferReader { + + @Override + public void transferCurrentValue(_Private_ByteTransferSink writer) throws IOException { + writer.writeBytes(buffer, (int) valuePreHeaderIndex, (int) (valueMarker.endIndex - valuePreHeaderIndex)); + } + } + @Override public T asFacet(Class facetType) { if (facetType == SpanProvider.class) { @@ -334,6 +343,11 @@ public T asFacet(Class facetType) { if (facetType == RawValueSpanProvider.class) { return facetType.cast(new RawValueSpanProviderFacet()); } + if (facetType == _Private_ByteTransferReader.class) { + if (!hasAnnotations && !isInStruct()) { + return facetType.cast(new ByteTransferReaderFacet()); + } + } } return null; } diff --git a/src/test/java/com/amazon/ion/impl/OptimizedBinaryWriterSymbolTableTest.java b/src/test/java/com/amazon/ion/impl/OptimizedBinaryWriterSymbolTableTest.java index ec2c379fe3..bc590ca292 100644 --- a/src/test/java/com/amazon/ion/impl/OptimizedBinaryWriterSymbolTableTest.java +++ b/src/test/java/com/amazon/ion/impl/OptimizedBinaryWriterSymbolTableTest.java @@ -22,6 +22,7 @@ import static com.amazon.ion.junit.IonAssert.assertIonIteratorEquals; import com.amazon.ion.IonDatagram; +import com.amazon.ion.IonReader; import com.amazon.ion.IonType; import com.amazon.ion.SymbolTable; import org.junit.Test; @@ -318,4 +319,95 @@ public void testOptimizedWriteValue_InterspersedReaderLSTs() assertIonEquals(expected, actual); } + @Test + public void testWriteValuesInStructWithSetFieldName() + throws Exception + { + // Multiple top-level values + byte[] multiValueDataBinary = encode("1 2 3 \"hello\" true"); + IonReader reader = system().newReader(multiValueDataBinary); + + iw = makeWriter(); + + // Start writing a struct + iw.stepIn(IonType.STRUCT); + + // Set field name for the struct's first child value + iw.setFieldName("values"); + + // The following fails on the second nested call to writeValue because no field name is set. + // The same field name is not applied to all values provided by the reader. + assertThrows(IllegalStateException.class, () -> iw.writeValues(reader)); + } + + @Test + public void testWriteValuesInStructWithoutSetFieldName() + throws Exception + { + // Struct with multiple values + byte[] multiValueDataBinary = encode("{foo: 1, bar: 2}"); + IonReader reader = system().newReader(multiValueDataBinary); + reader.next(); // Position the reader on the struct + + iw = makeWriter(); + + // Start writing a struct + iw.stepIn(IonType.STRUCT); + reader.stepIn(); // Enter the reader's struct + + // The field names from the reader's struct should be applied. + iw.writeValues(reader); + + // End the struct. + iw.stepOut(); + + IonDatagram expected = loader().load("{foo: 1, bar: 2}"); + IonDatagram actual = loader().load(outputByteArray()); + assertIonEquals(expected, actual); + } + + @Test + public void testWriteValuesToTopLevelFromStruct() + throws Exception + { + // Struct with multiple values + byte[] multiValueDataBinary = encode("{foo: 1, bar: 2}"); + IonReader reader = system().newReader(multiValueDataBinary); + reader.next(); // Position the reader on the struct + + iw = makeWriter(); + + reader.stepIn(); // Enter the reader's struct + + // The field names from the reader's struct should be dropped. + iw.writeValues(reader); + + IonDatagram expected = loader().load("1 2"); + IonDatagram actual = loader().load(outputByteArray()); + assertIonEquals(expected, actual); + } + + + @Test + public void testWriteValuesWithAnnotationSet() + throws Exception + { + // Multiple top-level annotated values + byte[] multiValueDataBinary = encode("b::1 c::2"); + IonReader reader = system().newReader(multiValueDataBinary); + reader.next(); // Position the reader on the struct + + iw = makeWriter(); + + // Set an annotation on the writer. This is expected to be clobbered. + iw.setTypeAnnotations("a"); + + // The annotations from the reader's struct should overwrite any set on the writer. + iw.writeValues(reader); + + IonDatagram expected = loader().load("b::1 c::2"); + IonDatagram actual = loader().load(outputByteArray()); + assertIonEquals(expected, actual); + } + } diff --git a/src/test/java/com/amazon/ion/impl/OptimizedBinaryWriterTestCase.java b/src/test/java/com/amazon/ion/impl/OptimizedBinaryWriterTestCase.java index c96fdbf25b..7ce4716eb5 100644 --- a/src/test/java/com/amazon/ion/impl/OptimizedBinaryWriterTestCase.java +++ b/src/test/java/com/amazon/ion/impl/OptimizedBinaryWriterTestCase.java @@ -180,10 +180,7 @@ protected void checkWriteValue(boolean expectedTransferInvoked) // TODO amazon-ion/ion-java/issues/16 - Currently, doesn't copy annotations or field names, // so we always expect no transfer of raw bytes - // TODO amazon-ion/ion-java/issues/381 - IonReaderBinaryIncremental currently doesn't support the byte transfer - // facet. Once this is fixed, remove the `asFacet` check below. - if (ir.isInStruct() || ir.getTypeAnnotationSymbols().length > 0 - || ir.asFacet(_Private_ByteTransferReader.class) == null) + if (ir.isInStruct() || ir.getTypeAnnotationSymbols().length > 0) { expectedTransferInvoked = false; } From b937612d293fc9cc0bb4500ed8f36bc9e1a323d9 Mon Sep 17 00:00:00 2001 From: Tyler Gregg Date: Tue, 16 Sep 2025 13:14:31 -0700 Subject: [PATCH 2/3] Don't implement byte transfer as a facet. --- .../IonReaderContinuableTopLevelBinary.java | 18 +++---- .../ion/impl/_Private_ByteTransferReader.java | 8 +++- .../ion/impl/bin/AbstractIonWriter.java | 15 +++--- .../impl/OptimizedBinaryWriterTestCase.java | 48 ++++--------------- 4 files changed, 28 insertions(+), 61 deletions(-) diff --git a/src/main/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java b/src/main/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java index 92557cded8..e2ee3165a3 100644 --- a/src/main/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java +++ b/src/main/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java @@ -45,7 +45,7 @@ * stream's values risk exceeding the available memory, then continuable reading must not be used. *

*/ -final class IonReaderContinuableTopLevelBinary extends IonReaderContinuableApplicationBinary implements IonReader, _Private_ReaderWriter { +final class IonReaderContinuableTopLevelBinary extends IonReaderContinuableApplicationBinary implements IonReader, _Private_ReaderWriter, _Private_ByteTransferReader { // True if continuable reading is disabled. private final boolean isNonContinuable; @@ -316,12 +316,13 @@ public void hoist(Span span) { } } - private class ByteTransferReaderFacet implements _Private_ByteTransferReader { - - @Override - public void transferCurrentValue(_Private_ByteTransferSink writer) throws IOException { - writer.writeBytes(buffer, (int) valuePreHeaderIndex, (int) (valueMarker.endIndex - valuePreHeaderIndex)); + @Override + public boolean transferCurrentValue(_Private_ByteTransferSink writer) throws IOException { + if (hasAnnotations || !isByteBacked() || isInStruct()) { + return false; } + writer.writeBytes(buffer, (int) valuePreHeaderIndex, (int) (valueMarker.endIndex - valuePreHeaderIndex)); + return true; } @Override @@ -343,11 +344,6 @@ public T asFacet(Class facetType) { if (facetType == RawValueSpanProvider.class) { return facetType.cast(new RawValueSpanProviderFacet()); } - if (facetType == _Private_ByteTransferReader.class) { - if (!hasAnnotations && !isInStruct()) { - return facetType.cast(new ByteTransferReaderFacet()); - } - } } return null; } diff --git a/src/main/java/com/amazon/ion/impl/_Private_ByteTransferReader.java b/src/main/java/com/amazon/ion/impl/_Private_ByteTransferReader.java index b1b9d95d35..df600751f9 100644 --- a/src/main/java/com/amazon/ion/impl/_Private_ByteTransferReader.java +++ b/src/main/java/com/amazon/ion/impl/_Private_ByteTransferReader.java @@ -24,6 +24,12 @@ */ public interface _Private_ByteTransferReader { - public void transferCurrentValue(_Private_ByteTransferSink writer) + /** + * Copies the raw bytes representing the current value, excluding any field name or annotations, if possible. + * @param writer the sink for the bytes + * @return true if the byte transfer occurred; false if it was not possible. + * @throws IOException if thrown by the sink during transfer. + */ + public boolean transferCurrentValue(_Private_ByteTransferSink writer) throws IOException; } diff --git a/src/main/java/com/amazon/ion/impl/bin/AbstractIonWriter.java b/src/main/java/com/amazon/ion/impl/bin/AbstractIonWriter.java index f8e1591163..ad0932b492 100644 --- a/src/main/java/com/amazon/ion/impl/bin/AbstractIonWriter.java +++ b/src/main/java/com/amazon/ion/impl/bin/AbstractIonWriter.java @@ -54,18 +54,15 @@ public final void writeValue(final IonReader reader) throws IOException { final IonType type = reader.getType(); - if (isStreamCopyOptimized()) + if (isStreamCopyOptimized() && reader instanceof _Private_ByteTransferReader) { - final _Private_ByteTransferReader transferReader = - reader.asFacet(_Private_ByteTransferReader.class); - - if (transferReader != null - && (_Private_Utils.isNonSymbolScalar(type) - || symtabExtendsCache.symtabsCompat(getSymbolTable(), reader.getSymbolTable()))) + if (_Private_Utils.isNonSymbolScalar(type) + || symtabExtendsCache.symtabsCompat(getSymbolTable(), reader.getSymbolTable())) { // we have something we can pipe over - transferReader.transferCurrentValue(this); - return; + if (((_Private_ByteTransferReader) reader).transferCurrentValue(this)) { + return; + } } } diff --git a/src/test/java/com/amazon/ion/impl/OptimizedBinaryWriterTestCase.java b/src/test/java/com/amazon/ion/impl/OptimizedBinaryWriterTestCase.java index 7ce4716eb5..faec85e6e1 100644 --- a/src/test/java/com/amazon/ion/impl/OptimizedBinaryWriterTestCase.java +++ b/src/test/java/com/amazon/ion/impl/OptimizedBinaryWriterTestCase.java @@ -46,7 +46,7 @@ public abstract class OptimizedBinaryWriterTestCase /** * Denotes whether the - * {@link _Private_ByteTransferReader#transferCurrentValue(IonWriterSystemBinary)} + * {@link _Private_ByteTransferReader#transferCurrentValue(_Private_ByteTransferSink)} * has been called after an {@link IonWriter#writeValue(IonReader)}. */ private boolean isTransferCurrentValueInvoked = false; @@ -107,60 +107,28 @@ protected byte[] outputByteArray() return bytes; } - private class TransferCurrentValueWatchingReader - implements _Private_ByteTransferReader - { - private final _Private_ByteTransferReader myDelegate; - - TransferCurrentValueWatchingReader(_Private_ByteTransferReader byteTransferReader) - { - myDelegate = byteTransferReader; - } - - public void transferCurrentValue(_Private_ByteTransferSink sink) - throws IOException - { - OptimizedBinaryWriterTestCase.this.isTransferCurrentValueInvoked = true; - myDelegate.transferCurrentValue(sink); - } - } - /** * Obtains a dynamic proxy of {@link IonReader} over the passed in byte[], * with an invocation handler hook over {@link _Private_ByteTransferReader} facet, - * so as to verify whether the transferCurrentValue() method is actually + * to verify whether the transferCurrentValue() method is actually * being called. - * - * @see TransferCurrentValueWatchingReader */ protected IonReader makeReaderProxy(byte[] bytes) { final IonReader reader = system().newReader(bytes); - InvocationHandler handler = new InvocationHandler() - { - public Object invoke(Object proxy, Method method, Object[] args) - throws Throwable + InvocationHandler handler = (proxy, method, args) -> { + if (method.getName().equals("transferCurrentValue")) { - if (method.getName().equals("asFacet") && - args.length == 1 && - args[0] == _Private_ByteTransferReader.class) - { - _Private_ByteTransferReader transferReader = - (_Private_ByteTransferReader) method.invoke(reader, args); - - if (transferReader == null) - return null; - - return new TransferCurrentValueWatchingReader(transferReader); - } - + OptimizedBinaryWriterTestCase.this.isTransferCurrentValueInvoked = (boolean) method.invoke(reader, args); + return isTransferCurrentValueInvoked; + } else { return method.invoke(reader, args); } }; return (IonReader) newProxyInstance(reader.getClass().getClassLoader(), - new Class[] { IonReader.class }, + new Class[] { IonReader.class, _Private_ByteTransferReader.class }, handler); } From 3a5681f6ec5240da8b7c8dd62ef2e12c4a33bd62 Mon Sep 17 00:00:00 2001 From: Tyler Gregg Date: Wed, 17 Sep 2025 17:01:03 -0700 Subject: [PATCH 3/3] Improves performance when stream copy optimization is enabled by not requiring symbol table copies. --- ...IonReaderContinuableApplicationBinary.java | 132 ++++++++++++++++++ .../IonReaderContinuableTopLevelBinary.java | 5 + .../com/amazon/ion/impl/LocalSymbolTable.java | 16 +++ .../ion/impl/_Private_ByteTransferReader.java | 10 ++ .../ion/impl/_Private_LocalSymbolTable.java | 26 +++- .../ion/impl/bin/AbstractIonWriter.java | 14 +- .../ion/impl/bin/IonManagedBinaryWriter.java | 31 +++- 7 files changed, 224 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/amazon/ion/impl/IonReaderContinuableApplicationBinary.java b/src/main/java/com/amazon/ion/impl/IonReaderContinuableApplicationBinary.java index 213c300415..332a9efd46 100644 --- a/src/main/java/com/amazon/ion/impl/IonReaderContinuableApplicationBinary.java +++ b/src/main/java/com/amazon/ion/impl/IonReaderContinuableApplicationBinary.java @@ -23,6 +23,7 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.HashMap; import java.util.Iterator; import java.util.List; @@ -36,6 +37,7 @@ import static com.amazon.ion.SystemSymbols.NAME_SID; import static com.amazon.ion.SystemSymbols.SYMBOLS_SID; import static com.amazon.ion.SystemSymbols.VERSION_SID; +import static com.amazon.ion.impl._Private_Utils.safeEquals; /** * An IonCursor capable of application-level parsing of binary Ion streams. @@ -82,6 +84,12 @@ class IonReaderContinuableApplicationBinary extends IonReaderContinuableCoreBina // symbol table is encountered in the stream. private SymbolTable cachedReadOnlySymbolTable = null; + // The cached SymbolTable that was determined to be a superset of the reader's current symbol table during a call + // to 'isSymbolTableSubsetOf'. This is set to null whenever the reader encounters a new symbol table. Therefore, + // when non-null, determining whether the reader's symbol table is a subset of a given table is as simple as + // checking whether that table is the same as 'lastSupersetSymbolTable'. + private SymbolTable lastSupersetSymbolTable = null; + // The reusable annotation iterator. private final AnnotationSequenceIterator annotationIterator = new AnnotationSequenceIterator(); @@ -206,6 +214,110 @@ private SymbolTable getSystemSymbolTable() { return SharedSymbolTable.getSystemSymbolTable(getIonMajorVersion()); } + boolean compareSymbolTableImportsArrayToList(SymbolTable[] arr, int arrayLength, List list) { + // Note: the array variant must begin with a system symbol table, while the list variant must not. + if (arrayLength - 1 != list.size()) { + return false; + } + for (int i = 1; i < arrayLength; i++) { + // TODO amazon-ion/ion-java/issues/18 Currently, we check imports by their references, which + // is overly strict; imports that have different references but the same symbols should pass the check. + // However, this is a cheaper check and is compatible with how common Catalog implementations handle + // shared tables. + if (list.get(i - 1) != arr[i]) { + return false; + } + } + return true; + } + + boolean compareSymbolsArrayToCollection(String[] arr, int arrayLength, Collection collection) { + // Precondition: the collection contains at least as many elements as the array. + Iterator collectionIterator = collection.iterator(); + for (int i = 0; i < arrayLength; i++) { + if (!safeEquals(arr[i], collectionIterator.next())) { + return false; + } + } + return true; + } + + /** + * Determines whether the symbol table active at the reader's current position is a subset of another symbol table, + * meaning that every symbol in the reader's symbol table is present and has the same symbol ID in the other + * table. + * @param other another symbol table. + * @return true if the reader's symbol table is a subset of the other table; otherwise, false. + */ + boolean isSymbolTableSubsetOf(SymbolTable other) + { + if (lastSupersetSymbolTable != null) { + // lastSupersetSymbolTable is reset when the reader's symbol table changes, so we know the reader's symbol + // table is the same as it was when last compared. This is an optimization that avoids the more expensive + // comparisons when this method is called repetitively within the same symbol table contexts. This + // commonly happens during repetitive calls to IonWriter.writeValue(IonReader), which is used directly + // by users and by the looping wrapper IonWriter.writeValues(IonReader). + return other == lastSupersetSymbolTable && other.getMaxId() == lastSupersetSymbolTable.getMaxId(); + } + + int numberOfLocalSymbols = localSymbolMaxOffset + 1; + int maxId = imports.getMaxId() + numberOfLocalSymbols; + + // Note: the first imported table is always the system symbol table. + boolean isSystemSymbolTable = numberOfLocalSymbols == 0 && imports.getImportedTablesNoCopy().length == 1; + boolean otherHasPrivateAttributes = other instanceof _Private_LocalSymbolTable; + _Private_LocalSymbolTable otherLocal = otherHasPrivateAttributes ? (_Private_LocalSymbolTable) other : null; + if (isSystemSymbolTable) { + if (other.isSystemTable() && maxId == other.getMaxId()) { + // Both represent the same system table. + lastSupersetSymbolTable = other; + return true; + } + // The other symbol table might not literally be the system symbol table, but if it's a local symbol table + // with zero local symbols and zero imports, that counts. + if (otherHasPrivateAttributes && otherLocal.getNumberOfLocalSymbols() == 0 && otherLocal.getImportedTablesAsList().isEmpty()) { + lastSupersetSymbolTable = other; + return true; + } + return false; + } + if (!otherHasPrivateAttributes) { + // The reader's symbol table is not a system symbol table, but the other is. Other cannot be a superset. + return false; + } + if (maxId > otherLocal.getMaxId()) return false; + + // NOTE: the following uses of _Private_LocalSymbolTable utilize knowledge of the implementation used by + // the binary writer, which has the only known use case for this method. Specifically, we call the interface + // method variants that return lists instead of arrays because we know this matches the binary writer's symbol + // table's internal representation and therefore does not require copying. If this method ends up being used + // for other symbol table implementations, which is unlikely, we should add logic to choose the most efficient + // variant to call for the particular implementation (such as by adding something like a `boolean usesArrays()` + // method to the interface). + + SymbolTable[] readerImports = imports.getImportedTablesNoCopy(); + if (!compareSymbolTableImportsArrayToList(readerImports, readerImports.length, otherLocal.getImportedTablesAsList())) { + return false; + } + + // Superset extends subset if subset doesn't have any declared symbols. + if (numberOfLocalSymbols == 0) { + lastSupersetSymbolTable = other; + return true; + } + + // Superset must have same/more declared (local) symbols than subset. + if (numberOfLocalSymbols > otherLocal.getNumberOfLocalSymbols()) return false; + + Collection otherSymbols = otherLocal.getLocalSymbolsNoCopy(); + if (!compareSymbolsArrayToCollection(symbols, numberOfLocalSymbols, otherSymbols)) { + return false; + } + + lastSupersetSymbolTable = other; + return true; + } + /** * Read-only snapshot of the local symbol table at the reader's current position. */ @@ -431,6 +543,21 @@ public _Private_LocalSymbolTable makeCopy() { public SymbolTable[] getImportedTablesNoCopy() { return importedTables.getImportedTablesNoCopy(); } + + @Override + public List getImportedTablesAsList() { + throw new UnsupportedOperationException("Call getImportedTablesNoCopy() instead."); + } + + @Override + public List getLocalSymbolsNoCopy() { + throw new UnsupportedOperationException("If this is needed, add a no-copy variant that returns an array."); + } + + @Override + public int getNumberOfLocalSymbols() { + return idToText.length; + } } /** @@ -442,6 +569,7 @@ private void resetSymbolTable() { Arrays.fill(symbols, 0, localSymbolMaxOffset + 1, null); localSymbolMaxOffset = -1; cachedReadOnlySymbolTable = null; + lastSupersetSymbolTable = null; } /** @@ -474,6 +602,7 @@ protected void restoreSymbolTable(SymbolTable symbolTable) { } localSymbolMaxOffset = snapshot.maxId - firstLocalSymbolId; System.arraycopy(snapshot.idToText, 0, symbols, 0, snapshot.idToText.length); + lastSupersetSymbolTable = null; } else { // Note: this will only happen when `symbolTable` is the system symbol table. resetSymbolTable(); @@ -626,6 +755,9 @@ private void finishReadingSymbolTableStruct() { } localSymbolMaxOffset += newSymbols.size(); } + // Note: last superset table is reset even if new symbols were simply appended because there's no + // guarantee those symbols are reflected in the superset table. + lastSupersetSymbolTable = null; state = State.READING_VALUE; } diff --git a/src/main/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java b/src/main/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java index e2ee3165a3..fd17cb1738 100644 --- a/src/main/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java +++ b/src/main/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinary.java @@ -325,6 +325,11 @@ public boolean transferCurrentValue(_Private_ByteTransferSink writer) throws IOE return true; } + @Override + public boolean isSymbolTableCompatible(SymbolTable symbolTable) { + return isSymbolTableSubsetOf(symbolTable); + } + @Override public T asFacet(Class facetType) { if (facetType == SpanProvider.class) { diff --git a/src/main/java/com/amazon/ion/impl/LocalSymbolTable.java b/src/main/java/com/amazon/ion/impl/LocalSymbolTable.java index 713d8998ae..22ac71689a 100644 --- a/src/main/java/com/amazon/ion/impl/LocalSymbolTable.java +++ b/src/main/java/com/amazon/ion/impl/LocalSymbolTable.java @@ -38,6 +38,7 @@ import com.amazon.ion.util.IonTextUtils; import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.Iterator; import java.util.List; @@ -610,6 +611,21 @@ public SymbolTable[] getImportedTablesNoCopy() return myImportsList.getImportedTablesNoCopy(); } + @Override + public List getImportedTablesAsList() { + throw new UnsupportedOperationException("Call getImportedTablesNoCopy() instead."); + } + + @Override + public Collection getLocalSymbolsNoCopy() { + throw new UnsupportedOperationException("If this is needed, add a no-copy variant that returns an array."); + } + + @Override + public int getNumberOfLocalSymbols() { + return mySymbolsCount; + } + public void writeTo(IonWriter writer) throws IOException { IonReader reader = new SymbolTableReader(this); diff --git a/src/main/java/com/amazon/ion/impl/_Private_ByteTransferReader.java b/src/main/java/com/amazon/ion/impl/_Private_ByteTransferReader.java index df600751f9..62e7dd4841 100644 --- a/src/main/java/com/amazon/ion/impl/_Private_ByteTransferReader.java +++ b/src/main/java/com/amazon/ion/impl/_Private_ByteTransferReader.java @@ -16,6 +16,8 @@ package com.amazon.ion.impl; import com.amazon.ion.IonReader; +import com.amazon.ion.SymbolTable; + import java.io.IOException; /** @@ -32,4 +34,12 @@ public interface _Private_ByteTransferReader */ public boolean transferCurrentValue(_Private_ByteTransferSink writer) throws IOException; + + /** + * Determines whether the reader's symbol table is compatible (i.e., a subset of) the given symbol table. When + * true, values can be transferred from the reader to the writer verbatim. + * @param symbolTable the symbol table active in the writer. + * @return true if the reader's symbol table is compatible; otherwise, false. + */ + public boolean isSymbolTableCompatible(SymbolTable symbolTable); } diff --git a/src/main/java/com/amazon/ion/impl/_Private_LocalSymbolTable.java b/src/main/java/com/amazon/ion/impl/_Private_LocalSymbolTable.java index f900fd0c83..fef4372360 100644 --- a/src/main/java/com/amazon/ion/impl/_Private_LocalSymbolTable.java +++ b/src/main/java/com/amazon/ion/impl/_Private_LocalSymbolTable.java @@ -2,7 +2,10 @@ import com.amazon.ion.SymbolTable; -interface _Private_LocalSymbolTable extends SymbolTable { +import java.util.Collection; +import java.util.List; + +public interface _Private_LocalSymbolTable extends SymbolTable { /** * @return a mutable copy of the symbol table. @@ -22,4 +25,25 @@ interface _Private_LocalSymbolTable extends SymbolTable { * @see SymbolTable#getImportedTables() */ SymbolTable[] getImportedTablesNoCopy(); + + /** + * Returns the imported symbol tables as a List without making a copy (if possible). + * Like {@link #getImportedTables()}, the list does not include the system symbol table. + * + * @return the imported symbol tables. Does not include the system symbol table. + * + * @see SymbolTable#getImportedTables() + */ + List getImportedTablesAsList(); + + /** + * Returns a collection containing the local symbols, without making a copy. + * @return the local symbols. + */ + Collection getLocalSymbolsNoCopy(); + + /** + * @return the number of local symbols, which do not include the imported or system symbols. + */ + int getNumberOfLocalSymbols(); } diff --git a/src/main/java/com/amazon/ion/impl/bin/AbstractIonWriter.java b/src/main/java/com/amazon/ion/impl/bin/AbstractIonWriter.java index ad0932b492..f8d2b1fe61 100644 --- a/src/main/java/com/amazon/ion/impl/bin/AbstractIonWriter.java +++ b/src/main/java/com/amazon/ion/impl/bin/AbstractIonWriter.java @@ -29,12 +29,11 @@ } /** The cache for copy optimization checks--null if not copy optimized. */ - private final _Private_SymtabExtendsCache symtabExtendsCache; + private final boolean isStreamCopyOptimized; /*package*/ AbstractIonWriter(final WriteValueOptimization optimization) { - this.symtabExtendsCache = optimization == WriteValueOptimization.COPY_OPTIMIZED - ? new _Private_SymtabExtendsCache() : null; + this.isStreamCopyOptimized = optimization == WriteValueOptimization.COPY_OPTIMIZED; } public final void writeValue(final IonValue value) throws IOException @@ -56,11 +55,10 @@ public final void writeValue(final IonReader reader) throws IOException if (isStreamCopyOptimized() && reader instanceof _Private_ByteTransferReader) { - if (_Private_Utils.isNonSymbolScalar(type) - || symtabExtendsCache.symtabsCompat(getSymbolTable(), reader.getSymbolTable())) + _Private_ByteTransferReader byteTransferReader = (_Private_ByteTransferReader) reader; + if (_Private_Utils.isNonSymbolScalar(type) || byteTransferReader.isSymbolTableCompatible(getSymbolTable())) { - // we have something we can pipe over - if (((_Private_ByteTransferReader) reader).transferCurrentValue(this)) { + if (byteTransferReader.transferCurrentValue(this)) { return; } } @@ -220,7 +218,7 @@ public final void writeTimestampUTC(final Date value) throws IOException public final boolean isStreamCopyOptimized() { - return symtabExtendsCache != null; + return isStreamCopyOptimized; } @SuppressWarnings("deprecation") diff --git a/src/main/java/com/amazon/ion/impl/bin/IonManagedBinaryWriter.java b/src/main/java/com/amazon/ion/impl/bin/IonManagedBinaryWriter.java index 61f5cbc170..5301aea327 100644 --- a/src/main/java/com/amazon/ion/impl/bin/IonManagedBinaryWriter.java +++ b/src/main/java/com/amazon/ion/impl/bin/IonManagedBinaryWriter.java @@ -37,6 +37,7 @@ import com.amazon.ion.SymbolToken; import com.amazon.ion.Timestamp; import com.amazon.ion.UnknownSymbolException; +import com.amazon.ion.impl._Private_LocalSymbolTable; import com.amazon.ion.impl.bin.IonRawBinaryWriter.StreamCloseMode; import com.amazon.ion.impl.bin.IonRawBinaryWriter.StreamFlushMode; import java.io.IOException; @@ -45,6 +46,7 @@ import java.math.BigInteger; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.Iterator; @@ -548,7 +550,7 @@ public void writeInt(IonManagedBinaryWriter self, BigInteger value) throws IOExc private static final SymbolTable[] EMPTY_SYMBOL_TABLE_ARRAY = new SymbolTable[0]; /** View over the internal local symbol table state as a symbol table. */ - private class LocalSymbolTableView extends AbstractSymbolTable + private class LocalSymbolTableView extends AbstractSymbolTable implements _Private_LocalSymbolTable { public LocalSymbolTableView() { @@ -637,6 +639,33 @@ public void makeReadOnly() { localsLocked = true; } + + @Override + public _Private_LocalSymbolTable makeCopy() { + throw new UnsupportedOperationException(); + } + + @Override + public SymbolTable[] getImportedTablesNoCopy() { + throw new UnsupportedOperationException("Call getImportedTablesAsList() instead."); + } + + @Override + public List getImportedTablesAsList() { + return imports.parents; + } + + @Override + public Collection getLocalSymbolsNoCopy() { + // Note: this is correct because `locals` is a LinkedHashMap, which orders keys in order of insertion. + // Therefore, the returned collection will return symbols in symbol ID order. + return locals.keySet(); + } + + @Override + public int getNumberOfLocalSymbols() { + return locals.size(); + } } private final IonCatalog catalog;