Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-7062169

(coll) micro-optimize ArrayList.remove(Object)

XMLWordPrintable

    • Icon: Enhancement Enhancement
    • Resolution: Fixed
    • Icon: P5 P5
    • 10
    • None
    • core-libs

      A DESCRIPTION OF THE REQUEST :
      AbstractList.java defines addAll as follows:

          public boolean addAll(int index, Collection<? extends E> c) {
              rangeCheckForAdd(index);
              boolean modified = false;
              for (E e : c) {
                  add(index++, e);
                  modified = true;
              }
              return modified;
          }

      with unnecessary 'modified' var; by definition of foreach, only case in which
      'modified == false' is when e.isEmpty; code should be replaced by shorted and
      faster version, e.g.

          public boolean addAll(int index, Collection<? extends E> c) {
              rangeCheckForAdd(index);
              for (E e : c)
                  add(index++, e);
              return !c.isEmpty();
          }

      ArrayList uses following code in some places (example of fastRemove here, but
      similar code is used in e.g. remove, addAll, removeRange etc.)

          private void fastRemove(int index) {
              modCount++;
              int numMoved = size - index - 1;
              if (numMoved > 0)
                  System.arraycopy(elementData, index+1, elementData, index,
                                   numMoved);
              elementData[--size] = null; // Let gc do its work
          }

      it can possibly be optimized to (clearer code, one less var initialized, less
      math done):

          private void fastRemove(int index) {
              modCount++;
              size--;
              if (size != index)
                  System.arraycopy(elementData, index+1, elementData, index, size -
      index);
              elementData[size] = null; // Let gc do its work
          }
      or, similarily (in removeRange):

              int numMoved = size - toIndex;
              System.arraycopy(elementData, toIndex, elementData, fromIndex,
                               numMoved);
      to just (numMoved is not used anywhere else)
              System.arraycopy(elementData, toIndex, elementData, fromIndex,
                               size - toIndex);

      JUSTIFICATION :
      Existing code creates performance losses with no apparent gain. Fixing the code increases performace and shortens the source code itself.

      EXPECTED VERSUS ACTUAL BEHAVIOR :
      EXPECTED -
      Using helper/temporary variables only when the value is indeed reused more than one time.
      ACTUAL -
      Creating one-shot temporaries without any reason.

      ---------- BEGIN SOURCE ----------
      see above
      ---------- END SOURCE ----------

      CUSTOMER SUBMITTED WORKAROUND :
      N/A; only creating own implementation of these classes solves the problem.

            martin Martin Buchholz
            webbuggrp Webbug Group
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved:
              Imported:
              Indexed: