diff --git a/src/com/amazon/ion/impl/IonReaderBinaryIncremental.java b/src/com/amazon/ion/impl/IonReaderBinaryIncremental.java index f5487b7dbc..90e1d7c840 100644 --- a/src/com/amazon/ion/impl/IonReaderBinaryIncremental.java +++ b/src/com/amazon/ion/impl/IonReaderBinaryIncremental.java @@ -34,6 +34,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Objects; /** *

@@ -101,6 +102,12 @@ private static class ContainerInfo { * The byte position of the end of the container. */ private int endPosition; + + private ContainerInfo initialize(final IonType type, final int endPosition) { + this.type = type; + this.endPosition = endPosition; + return this; + } } /** @@ -109,16 +116,6 @@ private static class ContainerInfo { private static final IonBufferConfiguration STANDARD_BUFFER_CONFIGURATION = IonBufferConfiguration.Builder.standard().build(); - // Constructs ContainerInfo instances. - private static final _Private_RecyclingStack.ElementFactory CONTAINER_INFO_FACTORY = - new _Private_RecyclingStack.ElementFactory() { - - @Override - public ContainerInfo newElement() { - return new ContainerInfo(); - } - }; - // Symbol IDs for symbols contained in the system symbol table. private static class SystemSymbolIDs { @@ -361,12 +358,13 @@ private static int logBase2(int value) { } lookahead = new IonReaderLookaheadBuffer(configuration, inputStream); buffer = (ResizingPipedInputStream) lookahead.getPipe(); - containerStack = new _Private_RecyclingStack( - CONTAINER_STACK_INITIAL_CAPACITY, - CONTAINER_INFO_FACTORY + containerStack = new _Private_RecyclingStack<>( + CONTAINER_STACK_INITIAL_CAPACITY, + ContainerInfo::new ); + annotationSids = new IntList(ANNOTATIONS_LIST_INITIAL_CAPACITY); - symbols = new ArrayList(SYMBOLS_LIST_INITIAL_CAPACITY); + symbols = new ArrayList<>(SYMBOLS_LIST_INITIAL_CAPACITY); scalarConverter = new _Private_ScalarConversions.ValueVariant(); resetImports(); } @@ -512,10 +510,7 @@ public boolean equals(Object o) { // NOTE: once ImportLocation is available via the SymbolToken interface, it should be compared here // when text is null. SymbolToken other = (SymbolToken) o; - if(getText() == null || other.getText() == null) { - return getText() == other.getText(); - } - return getText().equals(other.getText()); + return Objects.equals(getText(), other.getText()); } @Override @@ -567,8 +562,8 @@ private class LocalSymbolTableSnapshot implements SymbolTable, SymbolTableAsStru maxId = importsMaxId + numberOfLocalSymbols; // Map with initial size the number of symbols and load factor 1, meaning it must be full before growing. // It is not expected to grow. - listView = new ArrayList(symbols.subList(0, numberOfLocalSymbols)); - mapView = new HashMap((int) Math.ceil(numberOfLocalSymbols / 0.75), 0.75f); + listView = new ArrayList<>(symbols.subList(0, numberOfLocalSymbols)); + mapView = new HashMap<>((int) Math.ceil(numberOfLocalSymbols / 0.75), 0.75f); for (int i = 0; i < numberOfLocalSymbols; i++) { String symbol = listView.get(i); if (symbol != null) { @@ -853,7 +848,7 @@ private String getSymbol(int sid) { private SymbolToken getSymbolToken(int sid) { int symbolTableSize = maxSymbolId() + 1; if (symbolTokensById == null) { - symbolTokensById = new ArrayList(symbolTableSize); + symbolTokensById = new ArrayList<>(symbolTableSize); } if (symbolTokensById.size() < symbolTableSize) { for (int i = symbolTokensById.size(); i < symbolTableSize; i++) { @@ -908,7 +903,7 @@ private void readSymbolTable(IonReaderLookaheadBuffer.Marker marker) { peekIndex = currentValueEndPosition; } else if (typeID.type == IonType.LIST) { resetImports(); - newImports = new ArrayList(3); + newImports = new ArrayList<>(3); newImports.add(getSystemSymbolTable()); stepIn(); IonType type = next(); @@ -1204,9 +1199,7 @@ public void stepIn() { } // Note: the IonReader interface dictates that stepping into a null container has the same behavior as // an empty container. - ContainerInfo containerInfo = containerStack.push(); - containerInfo.type = valueType; - containerInfo.endPosition = valueEndPosition; + containerStack.push(c -> c.initialize(valueType, valueEndPosition)); valueType = null; valueTypeID = null; valueEndPosition = -1; diff --git a/src/com/amazon/ion/impl/_Private_RecyclingQueue.java b/src/com/amazon/ion/impl/_Private_RecyclingQueue.java index 5f015cd518..a52ccd7a35 100644 --- a/src/com/amazon/ion/impl/_Private_RecyclingQueue.java +++ b/src/com/amazon/ion/impl/_Private_RecyclingQueue.java @@ -20,6 +20,14 @@ public interface ElementFactory { T newElement(); } + @FunctionalInterface + public interface Recycler { + /** + * Re-initialize an element + */ + void recycle(T t); + } + /** * Iterator for the queue. */ @@ -67,7 +75,7 @@ public T get(int index) { * previously grown to the new depth. * @return the element at the top of the queue after the push. This element must be initialized by the caller. */ - public T push() { + public int push(Recycler recycler) { currentIndex++; if (currentIndex >= elements.size()) { top = elementFactory.newElement(); @@ -75,7 +83,8 @@ public T push() { } else { top = elements.get(currentIndex); } - return top; + recycler.recycle(top); + return currentIndex; } /** diff --git a/src/com/amazon/ion/impl/_Private_RecyclingStack.java b/src/com/amazon/ion/impl/_Private_RecyclingStack.java index a98e09d410..7f925cd2c3 100644 --- a/src/com/amazon/ion/impl/_Private_RecyclingStack.java +++ b/src/com/amazon/ion/impl/_Private_RecyclingStack.java @@ -2,13 +2,25 @@ import java.util.ArrayList; import java.util.List; +import java.util.ListIterator; +import java.util.NoSuchElementException; /** * A stack whose elements are recycled. This can be useful when the stack needs to grow and shrink * frequently and has a predictable maximum depth. * @param the type of elements stored. */ -public final class _Private_RecyclingStack { +public final class _Private_RecyclingStack implements Iterable { + private $Iterator stackIterator; + @Override + public ListIterator iterator() { + if (stackIterator != null) { + stackIterator.cursor = _Private_RecyclingStack.this.currentIndex; + } else { + stackIterator = new $Iterator(); + } + return stackIterator; + } /** * Factory for new stack elements. @@ -22,6 +34,14 @@ public interface ElementFactory { T newElement(); } + @FunctionalInterface + public interface Recycler { + /** + * Re-initialize an element + */ + void recycle(T t); + } + private final List elements; private final ElementFactory elementFactory; private int currentIndex; @@ -29,11 +49,11 @@ public interface ElementFactory { /** * @param initialCapacity the initial capacity of the underlying collection. - * @param elementFactory the factory used to create a new element on {@link #push()} when the stack has + * @param elementFactory the factory used to create a new element on {@link #push(Recycler)} when the stack has * not previously grown to the new depth. */ public _Private_RecyclingStack(int initialCapacity, ElementFactory elementFactory) { - elements = new ArrayList(initialCapacity); + elements = new ArrayList<>(initialCapacity); this.elementFactory = elementFactory; currentIndex = -1; top = null; @@ -44,7 +64,7 @@ public _Private_RecyclingStack(int initialCapacity, ElementFactory elementFac * previously grown to the new depth. * @return the element at the top of the stack after the push. This element must be initialized by the caller. */ - public T push() { + public T push(Recycler recycler) { currentIndex++; if (currentIndex >= elements.size()) { top = elementFactory.newElement(); @@ -52,6 +72,7 @@ public T push() { } else { top = elements.get(currentIndex); } + recycler.recycle(top); return top; } @@ -92,4 +113,62 @@ public boolean isEmpty() { public int size() { return currentIndex + 1; } + + private class $Iterator implements ListIterator { + private int cursor; + + @Override + public boolean hasNext() { + return cursor >= 0; + } + + @Override + public T next() { + if (!hasNext()) { + throw new NoSuchElementException(); + } + // post-decrement because "next" is where the cursor is + return _Private_RecyclingStack.this.elements.get(cursor--); + } + + @Override + public boolean hasPrevious() { + return cursor + 1 <= _Private_RecyclingStack.this.currentIndex; + } + + @Override + public T previous() { + if (!hasPrevious()) { + throw new NoSuchElementException(); + } + // pre-increment: "next" is where the cursor is, so "previous" is upward in stack + return _Private_RecyclingStack.this.elements.get(++cursor); + } + + @Override + public int nextIndex() { + return cursor; + } + + @Override + public int previousIndex() { + return cursor + 1; + } + + @Override + public void remove() { + throw new UnsupportedOperationException(); + } + + @Override + public void set(T t) { + throw new UnsupportedOperationException(); + } + + @Override + public void add(T t) { + throw new UnsupportedOperationException(); + } + } + } diff --git a/src/com/amazon/ion/impl/bin/IonRawBinaryWriter.java b/src/com/amazon/ion/impl/bin/IonRawBinaryWriter.java index b85ea74fcb..88e4cca7e3 100644 --- a/src/com/amazon/ion/impl/bin/IonRawBinaryWriter.java +++ b/src/com/amazon/ion/impl/bin/IonRawBinaryWriter.java @@ -55,10 +55,8 @@ import java.io.OutputStream; import java.math.BigDecimal; import java.math.BigInteger; -import java.util.ArrayList; import java.util.Iterator; -import java.util.List; -import java.util.NoSuchElementException; +import java.util.ListIterator; /** * Low-level binary {@link IonWriter} that understands encoding concerns but doesn't operate with any sense of symbol table management. @@ -258,37 +256,42 @@ private class ContainerInfo /** The size of the current value. */ public long length; /** - * The index of the patch point placeholder. + * The index of the patch point if present, -1 otherwise. */ - public int placeholderPatchIndex; + public int patchIndex; public ContainerInfo() { type = null; position = -1; length = -1; - placeholderPatchIndex = -1; + patchIndex = -1; } public void appendPatch(final long oldPosition, final int oldLength, final long length) { - patchPoints.get(placeholderPatchIndex).initialize(oldPosition, oldLength, length); + if (patchIndex == -1) { + // We have no assigned patch point, we need to make our own + patchIndex = patchPoints.push(p -> p.initialize(oldPosition, oldLength, length)); + } else { + // We have an assigned patch point already, but we need to overwrite it with the correct data + patchPoints.get(patchIndex).initialize(oldPosition, oldLength, length); + } } - public void initialize(final ContainerType type, final long offset) { + public ContainerInfo initialize(final ContainerType type, final long offset) { this.type = type; this.position = offset; this.length = 0; - if (type != ContainerType.VALUE) { - placeholderPatchIndex = patchPoints.size(); - patchPoints.push().initialize(-1, -1, -1); - } + this.patchIndex = -1; + + return this; } @Override public String toString() { - return "(CI " + type + " pos:" + position + " len:" + length + ")"; + return "(CI " + type + " pos:" + position + " len:" + length + " patch:"+patchIndex+")"; } } @@ -313,10 +316,15 @@ public String toString() return "(PP old::(" + oldPosition + " " + oldLength + ") patch::(" + length + ")"; } - public void initialize(final long oldPosition, final int oldLength, final long length) { + public PatchPoint initialize(final long oldPosition, final int oldLength, final long length) { this.oldPosition = oldPosition; this.oldLength = oldLength; this.length = length; + return this; + } + + public PatchPoint clear() { + return initialize(-1, -1, -1); } } @@ -391,7 +399,6 @@ public ContainerInfo newElement() { this.currentFieldSid = SID_UNASSIGNED; this.currentAnnotationSids = new IntList(); this.hasTopLevelSymbolTableAnnotation = false; - this.closed = false; } @@ -543,23 +550,30 @@ private void updateLength(long length) private void pushContainer(final ContainerType type) { // XXX we push before writing the type of container - containers.push().initialize(type, buffer.position() + 1); + containers.push(c -> c.initialize(type, buffer.position() + 1)); } - private void addPatchPoint(final long position, final int oldLength, final long value, final ContainerInfo container) + private void addPatchPoint(final ContainerInfo container, final long position, final int oldLength, final long value) { + // If we're adding a patch point we first need to ensure that all of our ancestors (containing values) already + // have a patch point. No container can be smaller than the contents, so all outer layers also require patches. + // Instead of allocating iterator, we share one iterator instance within the scope of the container stack and reset the cursor every time we track back to the ancestors. + ListIterator stackIterator = containers.iterator(); + // Walk down the stack until we find an ancestor which already has a patch point + while (stackIterator.hasNext() && stackIterator.next().patchIndex == -1); + + // The iterator cursor is now positioned on an ancestor container that has a patch point + // Ascend back up the stack, fixing the ancestors which need a patch point assigned before us + while (stackIterator.hasPrevious()) { + ContainerInfo ancestor = stackIterator.previous(); + if (ancestor.patchIndex == -1) { + ancestor.patchIndex = patchPoints.push(PatchPoint::clear); + } + } + // record the size of the length data. final int patchLength = WriteBuffer.varUIntLength(value); - if (container == null) - { - // not nested, just append to the root list - patchPoints.push().initialize(position, oldLength, value); - } - else - { - // nested, apply it to the current container - container.appendPatch(position, oldLength, value); - } + container.appendPatch(position, oldLength, value); updateLength(patchLength - oldLength); } @@ -613,7 +627,6 @@ private ContainerInfo popContainer() // We've reclaimed some number of bytes; adjust the container length as appropriate. length -= numberOfBytesToShiftBy; - reclaimPlaceholderPatchPoint(currentContainer.placeholderPatchIndex); } else if (currentContainer.length <= preallocationMode.contentMaxLength) { @@ -621,14 +634,13 @@ else if (currentContainer.length <= preallocationMode.contentMaxLength) // fit in the preallocated length bytes that were added to the buffer when the container was started. // Update those bytes with the VarUInt encoding of the length value. preallocationMode.patchLength(buffer, positionOfFirstLengthByte, length); - reclaimPlaceholderPatchPoint(currentContainer.placeholderPatchIndex); } else { // The container's encoded body is too long to fit in the length bytes that were preallocated. - // Write the VarUInt encoding of the length in a secondary buffer and make a note to include that - // when we go to flush the primary buffer to the output stream. - addPatchPoint(positionOfFirstLengthByte, preallocationMode.numberOfLengthBytes(), length, currentContainer); + // Write the length in the patch point list, making a note to include the VarUInt encoding of the length + // at the right point when we go to flush the primary buffer to the output stream. + addPatchPoint(currentContainer, positionOfFirstLengthByte, preallocationMode.numberOfLengthBytes(), length); } } @@ -1074,7 +1086,7 @@ private void patchSingleByteTypedOptimisticValue(final byte type, final Containe { // side patch buffer.writeUInt8At(info.position - 1, type | 0xE); - addPatchPoint(info.position, 0, info.length, null); + addPatchPoint(info, info.position, 0, info.length); } }