Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 18 additions & 25 deletions src/com/amazon/ion/impl/IonReaderBinaryIncremental.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;

/**
* <p>
Expand Down Expand Up @@ -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;
}
}

/**
Expand All @@ -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<ContainerInfo> CONTAINER_INFO_FACTORY =
new _Private_RecyclingStack.ElementFactory<ContainerInfo>() {

@Override
public ContainerInfo newElement() {
return new ContainerInfo();
}
};

// Symbol IDs for symbols contained in the system symbol table.
private static class SystemSymbolIDs {

Expand Down Expand Up @@ -361,12 +358,13 @@ private static int logBase2(int value) {
}
lookahead = new IonReaderLookaheadBuffer(configuration, inputStream);
buffer = (ResizingPipedInputStream) lookahead.getPipe();
containerStack = new _Private_RecyclingStack<ContainerInfo>(
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<String>(SYMBOLS_LIST_INITIAL_CAPACITY);
symbols = new ArrayList<>(SYMBOLS_LIST_INITIAL_CAPACITY);
scalarConverter = new _Private_ScalarConversions.ValueVariant();
resetImports();
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<String>(symbols.subList(0, numberOfLocalSymbols));
mapView = new HashMap<String, Integer>((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) {
Expand Down Expand Up @@ -853,7 +848,7 @@ private String getSymbol(int sid) {
private SymbolToken getSymbolToken(int sid) {
int symbolTableSize = maxSymbolId() + 1;
if (symbolTokensById == null) {
symbolTokensById = new ArrayList<SymbolToken>(symbolTableSize);
symbolTokensById = new ArrayList<>(symbolTableSize);
}
if (symbolTokensById.size() < symbolTableSize) {
for (int i = symbolTokensById.size(); i < symbolTableSize; i++) {
Expand Down Expand Up @@ -908,7 +903,7 @@ private void readSymbolTable(IonReaderLookaheadBuffer.Marker marker) {
peekIndex = currentValueEndPosition;
} else if (typeID.type == IonType.LIST) {
resetImports();
newImports = new ArrayList<SymbolTable>(3);
newImports = new ArrayList<>(3);
newImports.add(getSystemSymbolTable());
stepIn();
IonType type = next();
Expand Down Expand Up @@ -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));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a change that would warrant performance testing of binary incremental reading to ensure it does not introduce a regression. Do we see extra overhead introduced by creation and invocation of a lambda on each stepIn?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will check if the change introduce regression on binary incremental reading.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the benchmark results of benchmarking incremental reader with 4 test data, and the results are neutral.

Test Data Before After
log_59155.ion 242.721 249.478
log_194617.ion 1858.668 1854.447
test.json 4273.663 4266.974
catalog.ion 0.476 0.478
singleValue.10n 0.063 0.063

valueType = null;
valueTypeID = null;
valueEndPosition = -1;
Expand Down
13 changes: 11 additions & 2 deletions src/com/amazon/ion/impl/_Private_RecyclingQueue.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ public interface ElementFactory<T> {
T newElement();
}

@FunctionalInterface
public interface Recycler<T> {
/**
* Re-initialize an element
*/
void recycle(T t);
}

/**
* Iterator for the queue.
*/
Expand Down Expand Up @@ -67,15 +75,16 @@ 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<T> recycler) {
currentIndex++;
if (currentIndex >= elements.size()) {
top = elementFactory.newElement();
elements.add(top);
} else {
top = elements.get(currentIndex);
}
return top;
recycler.recycle(top);
return currentIndex;
}

/**
Expand Down
87 changes: 83 additions & 4 deletions src/com/amazon/ion/impl/_Private_RecyclingStack.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 <T> the type of elements stored.
*/
public final class _Private_RecyclingStack<T> {
public final class _Private_RecyclingStack<T> implements Iterable<T> {
private $Iterator stackIterator;
@Override
public ListIterator<T> iterator() {
if (stackIterator != null) {
stackIterator.cursor = _Private_RecyclingStack.this.currentIndex;
} else {
stackIterator = new $Iterator();
}
return stackIterator;
}

/**
* Factory for new stack elements.
Expand All @@ -22,18 +34,26 @@ public interface ElementFactory<T> {
T newElement();
}

@FunctionalInterface
public interface Recycler<T> {
/**
* Re-initialize an element
*/
void recycle(T t);
}

private final List<T> elements;
private final ElementFactory<T> elementFactory;
private int currentIndex;
private T top;

/**
* @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<T> elementFactory) {
elements = new ArrayList<T>(initialCapacity);
elements = new ArrayList<>(initialCapacity);
this.elementFactory = elementFactory;
currentIndex = -1;
top = null;
Expand All @@ -44,14 +64,15 @@ public _Private_RecyclingStack(int initialCapacity, ElementFactory<T> 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<T> recycler) {
currentIndex++;
if (currentIndex >= elements.size()) {
top = elementFactory.newElement();
elements.add(top);
} else {
top = elements.get(currentIndex);
}
recycler.recycle(top);
return top;
}

Expand Down Expand Up @@ -92,4 +113,62 @@ public boolean isEmpty() {
public int size() {
return currentIndex + 1;
}

private class $Iterator implements ListIterator<T> {
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();
}
}

}
Loading