Extract Method: isolate END_ELEMENT handling in computeNext()

[EmanuelAry]

Overview

The DexClassData#computeNext() method had grown into a large, monolithic loop that interleaved handling of START_ELEMENT and END_ELEMENT events, making it hard to follow. This refactoring extracts the entire END_ELEMENT branch into a dedicated handleEndElement() method.

To safely manage the multiple mutable state variables (currentClass, currentMethod, inConstructor) and support an early return when a class ends, a small result holder class EndElementResult was introduced. The main loop now delegates the END_ELEMENT case, updates its local state from the result, and returns immediately if the end-of-class condition is met.

Proposed Changes

• Added private method handleEndElement(...) containing all logic previously in the case END_ELEMENT block.
• Added private static class EndElementResult to hold updated builder, method, constructor flag, and optional return value.
• Simplified the main while loop: END_ELEMENT now only calls the extracted method and processes the result.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[gemini-code-assist]

Code Review

This pull request refactors DexDumpIterator.java by extracting the END_ELEMENT processing logic from computeNext() into a new helper method handleEndElement, utilizing a new helper class EndElementResult to manage state. The reviewer recommended avoiding the creation of EndElementResult to prevent excessive object allocations and garbage collection overhead when parsing large XML files. Instead, they suggested promoting the local state variables to instance variables of DexDumpIterator, which simplifies the helper method signatures and improves both performance and code readability.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Performance & Code Quality Improvement: Eliminate EndElementResult Allocations

The DexDumpIterator class is designed to parse very large XML files (~10-100MB) efficiently, as noted in the class-level JAVADOC. Introducing a new EndElementResult object for every single END_ELEMENT event (which can occur millions of times in a large file) introduces significant and unnecessary garbage collection pressure.

Instead of creating a temporary result holder class to pass mutable state back and forth, we can promote currentClass, currentMethod, annotationStack, and inConstructor to instance variables of DexDumpIterator (similar to how currentPackage and currentLocation are already managed).

This refactoring provides several key benefits:

1. Performance: Completely eliminates millions of short-lived EndElementResult allocations, significantly reducing GC overhead.
2. Simplicity: Simplifies the signature of handleEndElement (and potentially other helper methods) as they no longer need to accept or return these state variables.
3. Readability & Style: The main loop in computeNext() becomes much cleaner, and we can restore the consistent 2-space indentation used throughout the rest of the file.

  private DexClassData.Builder currentClass = null;
  private MethodData.MethodBuilder currentMethod = null;
  private final Deque<AnnotationContext> annotationStack = Lists.newLinkedList();
  private boolean inConstructor = false;

  @Override
  protected DexClassData computeNext() {
    while (streamHasNext()) {
      String name = null;
      switch (streamNext()) {
        case END_DOCUMENT:
          return endOfData();
        case START_ELEMENT:
          currentLocation = xmlStreamReader.getLocation();
          name = xmlStreamReader.getLocalName();
          if ("package".equals(name)) {
            currentPackage = handleNewPackageNode();
          } else if ("class".equals(name)) {
            currentClass = handleNewClassNode(currentClass);
          } else if ("method".equals(name)) {
            currentMethod = handleNewMethodNode(currentMethod);
          } else if ("parameter".equals(name)) {
            currentMethod = handleParameterNode(currentMethod, inConstructor);
          } else if ("constructor".equals(name)) {
            checkState(!inConstructor, "Already in a constructor node. %s",
                currentLocation);
            inConstructor = true;
          } else if ("annotation".equals(name)) {
            AnnotationContext context = new AnnotationContext();
            annotationStack.push(context);
            context.currentAnnotation = handleNewAnnotationNode();
          } else if ("anno_field".equals(name)) {
            checkState(!annotationStack.isEmpty(), "Not in annotation! %s",
                currentLocation);
            AnnotationContext context = annotationStack.pop();
            context.currentAnnotationValue = handleNewAnnotationFieldNode(
                context.currentAnnotationValue);
            annotationStack.push(context);
          } else if ("anno_field_value".equals(name)) {
            checkState(!annotationStack.isEmpty(), "Not in annotation! %s",
                currentLocation);
            AnnotationContext context = annotationStack.pop();
            context.currentAnnotationValue = handleNewAnnotationFieldValueNode(
                context.currentAnnotationValue,
                context.currentArrayStatus);
            annotationStack.push(context);
          } else if ("anno_field_array".equals(name)) {
            checkState(!annotationStack.isEmpty(), "Not in annotation! %s",
                currentLocation);
            AnnotationContext context = annotationStack.pop();
            context.currentArrayStatus = handleNewAnnotationFieldArrayNode(
                context.currentAnnotationValue,
                context.currentArrayStatus);
            annotationStack.push(context);
          } else if ("array_element".equals(name)) {
            checkState(!annotationStack.isEmpty(), "Not in annotation! %s",
                currentLocation);
            AnnotationContext context = annotationStack.pop();
            context.currentArrayStatus = handleNewAnnotationFieldArrayElementNode(
                context.currentAnnotationValue,
                context.currentArrayStatus);
            annotationStack.push(context);
          }
          break;
        case END_ELEMENT:
          currentLocation = xmlStreamReader.getLocation();
          name = xmlStreamReader.getLocalName();
          DexClassData classData = handleEndElement(name);
          if (classData != null) {
            return classData;
          }
          break;
        default: { /* don't care */ }
      }
      currentLocation = null;
    }
    throw new IllegalStateException("Should never happen. Document end not emitted?");
  }

  private DexClassData handleEndElement(String name) {
    if ("package".equals(name)) {
      currentPackage = handleEndPackageNode();
    } else if ("class".equals(name)) {
      DexClassData classData = handleEndClassNode(currentClass, currentMethod, annotationStack);
      currentClass = null;
      return classData;
    } else if ("method".equals(name)) {
      currentMethod = handleEndMethodNode(currentClass, currentMethod);
    } else if ("constructor".equals(name)) {
      checkState(inConstructor, "Not in constructor. %s", currentLocation);
      inConstructor = false;
    } else if ("annotation".equals(name)) {
      checkState(!annotationStack.isEmpty(), "Not in annotation! %s", currentLocation);
      AnnotationContext context = annotationStack.pop();
      checkState(null == context.currentAnnotationValue, "Never closed value node %s",
          currentLocation);
      checkState(null == context.currentArrayStatus, "Never closed array node %s",
          currentLocation);

      if (annotationStack.isEmpty()) {
        handleEndAnnotationNode(
            context.currentAnnotation,
            currentClass,
            currentMethod,
            inConstructor);
      } else {
        AnnotationContext parentContext = annotationStack.pop();
        AnnotationPb nestedAnnotation =
            handleEndNestedAnnotation(
                context.currentAnnotation,
                parentContext.currentAnnotation,
                parentContext.currentAnnotationValue);
        parentContext.nestedAnnotations.add(nestedAnnotation);
        annotationStack.push(parentContext);
      }
    } else if ("anno_field".equals(name)) {
      checkState(!annotationStack.isEmpty(), "Not in annotation! %s", currentLocation);
      AnnotationContext context = annotationStack.pop();
      context.currentAnnotationValue = handleEndAnnotationFieldNode(
          context.currentAnnotation,
          context.currentAnnotationValue,
          context.nestedAnnotations);
      context.nestedAnnotations.clear();
      annotationStack.push(context);
    } else if ("anno_field_value".equals(name)) {
      checkState(!annotationStack.isEmpty(), "Not in annotation! %s", currentLocation);
      AnnotationContext context = annotationStack.pop();
      checkState(null != context.currentAnnotationValue, "anno value not set. %s",
          currentLocation);
      annotationStack.push(context);
    } else if ("anno_field_array".equals(name)) {
      checkState(!annotationStack.isEmpty(), "Not in annotation! %s", currentLocation);
      AnnotationContext context = annotationStack.pop();
      context.currentArrayStatus = handleEndAnnotationFieldArrayNode(
          context.currentAnnotationValue,
          context.currentArrayStatus);
      annotationStack.push(context);
    } else if ("array_element".equals(name)) {
      checkState(!annotationStack.isEmpty(), "Not in annotation! %s", currentLocation);
      AnnotationContext context = annotationStack.pop();
      context.currentArrayStatus = handleEndAnnotationFieldArrayElementNode(
          context.currentAnnotationValue,
          context.currentArrayStatus);
      annotationStack.push(context);
    }
    return null;
  }