Iteratively skip containers in Ion Text reader (#535)

* Iteratively skip containers in Ion Text reader * Reduce allocations by moving arraylist into class, and fix preformance testing workflow
amazon-ion · Jul 20, 2023 · 3fb88a1 · 3fb88a1
1 parent 68365e0
commit 3fb88a1
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 4 deletions.
diff --git a/.github/workflows/ion-java-performance-regression-detector.yml b/.github/workflows/ion-java-performance-regression-detector.yml
@@ -19,6 +19,7 @@ jobs:
       - name: Set up JDK 11
         uses: actions/setup-java@v3
         with:
+          distribution: 'corretto'
           java-version: 11
 
       - name: Checkout ion-java from the new commit.

diff --git a/src/com/amazon/ion/impl/IonReaderTextRawTokensX.java b/src/com/amazon/ion/impl/IonReaderTextRawTokensX.java
@@ -30,6 +30,7 @@
 import com.amazon.ion.impl.UnifiedSavePointManagerX.SavePoint;
 import com.amazon.ion.util.IonTextUtils;
 import java.io.IOException;
+import java.util.ArrayList;
 
 /**
  * Tokenizer for the Ion text parser in IonTextIterator. This
@@ -107,6 +108,10 @@ public Appendable append(char c) throws IOException
      */
     private int                 _base64_prefetch_stack;
 
+    // This value was chosen somewhat arbitrarily; it can/should be changed if it is found to be insufficient.
+    private static final int CONTAINER_STACK_INITIAL_CAPACITY = 16;
+    // Used for tracking terminator characters when skipping a container
+    private final ArrayList<Integer> containerSkipTerminatorStack = new ArrayList<>(CONTAINER_STACK_INITIAL_CAPACITY);
 
     /**
      * IonTokenReader constructor requires a UnifiedInputStream
@@ -1206,6 +1211,11 @@ protected void skip_over_sexp() throws IOException
     private void skip_over_container(int terminator) throws IOException
     {
         assert( terminator == '}' || terminator == ']' || terminator == ')' );
+
+        // In theory, this should be empty at the start and end of every call to this
+        // method, but we'll clear it here anyway just in case.
+        containerSkipTerminatorStack.clear();
+
         int c;
 
         for (;;) {
@@ -1217,7 +1227,12 @@ private void skip_over_container(int terminator) throws IOException
             case ']':
             case ')':
                 if (c == terminator) { // no point is checking this on every char
-                    return;
+                    if (containerSkipTerminatorStack.isEmpty()) {
+                        return;
+                    } else {
+                        // Pop one off the stack to continue
+                        terminator = containerSkipTerminatorStack.remove(containerSkipTerminatorStack.size() - 1);
+                    }
                 }
                 break;
             case '"':
@@ -1233,10 +1248,12 @@ private void skip_over_container(int terminator) throws IOException
                 }
                 break;
             case '(':
-                skip_over_container(')');
+                containerSkipTerminatorStack.add(terminator);
+                terminator = ')';
                 break;
             case '[':
-                skip_over_container(']');
+                containerSkipTerminatorStack.add(terminator);
+                terminator = ']';
                 break;
             case '{':
                 // this consumes lobs as well since the double
@@ -1273,7 +1290,8 @@ else if (c == '}') {
                 }
                 else {
                     unread_char(c);
-                    skip_over_container('}');
+                    containerSkipTerminatorStack.add(terminator);
+                    terminator = '}';
                 }
                 break;
             default: