Improves performance when stream copy optimization is enabled by not requiring symbol table copies.

Author: tgregg

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<SymbolTable> 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<String> collection) {
+        // Precondition: the collection contains at least as many elements as the array.
+        Iterator<String> 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<String> 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<SymbolTable> getImportedTablesAsList() {
+            throw new UnsupportedOperationException("Call getImportedTablesNoCopy() instead.");
+        }
+
+        @Override
+        public List<String> 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;
         }
 

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> T asFacet(Class<T> facetType) {
         if (facetType == SpanProvider.class) {

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<SymbolTable> getImportedTablesAsList() {
+        throw new UnsupportedOperationException("Call getImportedTablesNoCopy() instead.");
+    }
+
+    @Override
+    public Collection<String> 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);

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);
 }

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<SymbolTable> getImportedTablesAsList();
+
+    /**
+     * Returns a collection containing the local symbols, without making a copy.
+     * @return the local symbols.
+     */
+    Collection<String> getLocalSymbolsNoCopy();
+
+    /**
+     * @return the number of local symbols, which do not include the imported or system symbols.
+     */
+    int getNumberOfLocalSymbols();
 }

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")

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<SymbolTable> getImportedTablesAsList() {
+            return imports.parents;
+        }
+
+        @Override
+        public Collection<String> 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;