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

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

    XMLWordPrintable

Details

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

    Description

      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.

      Attachments

        Issue Links

          Activity

            People

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

              Dates

                Created:
                Updated:
                Resolved:
                Imported:
                Indexed: