Return-Path: Delivered-To: apmail-harmony-commits-archive@www.apache.org Received: (qmail 24901 invoked from network); 12 Aug 2010 14:07:52 -0000 Received: from unknown (HELO mail.apache.org) (140.211.11.3) by 140.211.11.9 with SMTP; 12 Aug 2010 14:07:52 -0000 Received: (qmail 13954 invoked by uid 500); 12 Aug 2010 14:07:52 -0000 Delivered-To: apmail-harmony-commits-archive@harmony.apache.org Received: (qmail 13857 invoked by uid 500); 12 Aug 2010 14:07:50 -0000 Mailing-List: contact commits-help@harmony.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@harmony.apache.org Delivered-To: mailing list commits@harmony.apache.org Received: (qmail 13848 invoked by uid 99); 12 Aug 2010 14:07:49 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 12 Aug 2010 14:07:49 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=10.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 12 Aug 2010 14:07:48 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id 079A1238890D; Thu, 12 Aug 2010 14:06:32 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r984789 - in /harmony/enhanced/java/branches/java6: ./ classlib/ classlib/depends/libs/ classlib/modules/luni/src/main/java/java/util/ArrayList.java drlvm/ jdktools/ Date: Thu, 12 Aug 2010 14:06:31 -0000 To: commits@harmony.apache.org From: hindessm@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20100812140632.079A1238890D@eris.apache.org> Author: hindessm Date: Thu Aug 12 14:06:31 2010 New Revision: 984789 URL: http://svn.apache.org/viewvc?rev=984789&view=rev Log: Merge change from /harmony/enhanced/java/trunk@984779: r984779 | hindessm | 2010-08-12 14:53:35 +0100 (Thu, 12 Aug 2010) | 3 lines Remove some REVIEW comments and minor cleanup after discussion with Oliver. Modified: harmony/enhanced/java/branches/java6/ (props changed) harmony/enhanced/java/branches/java6/classlib/ (props changed) harmony/enhanced/java/branches/java6/classlib/depends/libs/ (props changed) harmony/enhanced/java/branches/java6/classlib/modules/luni/src/main/java/java/util/ArrayList.java harmony/enhanced/java/branches/java6/drlvm/ (props changed) harmony/enhanced/java/branches/java6/jdktools/ (props changed) Propchange: harmony/enhanced/java/branches/java6/ ------------------------------------------------------------------------------ --- svn:mergeinfo (original) +++ svn:mergeinfo Thu Aug 12 14:06:31 2010 @@ -1,4 +1,4 @@ /harmony/enhanced/java/branches/mrh:935751-941490 -/harmony/enhanced/java/trunk:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731 +/harmony/enhanced/java/trunk:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731,984779 /harmony/enhanced/trunk:476395-929252 /incubator/harmony/enhanced/trunk:292550-476394 Propchange: harmony/enhanced/java/branches/java6/classlib/ ------------------------------------------------------------------------------ --- svn:mergeinfo (original) +++ svn:mergeinfo Thu Aug 12 14:06:31 2010 @@ -1,7 +1,7 @@ /harmony/enhanced/classlib/trunk:713674-735919,765923-926091,926318-926838 /harmony/enhanced/classlib/trunk/working_classlib:884014-884286 /harmony/enhanced/java/branches/mrh/classlib:935751-941490 -/harmony/enhanced/java/trunk/classlib:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731 +/harmony/enhanced/java/trunk/classlib:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731,984779 /harmony/enhanced/trunk/classlib:476395-929252 /harmony/enhanced/trunk/working_classlib:476396-920147 /incubator/harmony/enhanced/trunk/classlib:292550-476394 Propchange: harmony/enhanced/java/branches/java6/classlib/depends/libs/ ------------------------------------------------------------------------------ --- svn:mergeinfo (original) +++ svn:mergeinfo Thu Aug 12 14:06:31 2010 @@ -1,4 +1,4 @@ /harmony/enhanced/classlib/trunk/depends/libs:544451-926091 -/harmony/enhanced/java/trunk/classlib/depends/libs:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731 +/harmony/enhanced/java/trunk/classlib/depends/libs:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731,984779 /harmony/enhanced/trunk/classlib/depends/libs:476395-929252 /incubator/harmony/enhanced/trunk/classlib/depends/libs:292550-476394 Modified: harmony/enhanced/java/branches/java6/classlib/modules/luni/src/main/java/java/util/ArrayList.java URL: http://svn.apache.org/viewvc/harmony/enhanced/java/branches/java6/classlib/modules/luni/src/main/java/java/util/ArrayList.java?rev=984789&r1=984788&r2=984789&view=diff ============================================================================== --- harmony/enhanced/java/branches/java6/classlib/modules/luni/src/main/java/java/util/ArrayList.java (original) +++ harmony/enhanced/java/branches/java6/classlib/modules/luni/src/main/java/java/util/ArrayList.java Thu Aug 12 14:06:31 2010 @@ -120,7 +120,6 @@ public class ArrayList extends Abstra } array[--firstIndex] = object; } else if (location == size) { - // REVIEW: Why not just call add()? Matching RI behaviour? if (firstIndex + size == array.length) { growAtEnd(1); } @@ -128,10 +127,8 @@ public class ArrayList extends Abstra } else { // must be case: (0 < location && location < size) if (size == array.length) { growForInsert(location, 1); - } else if ((location < size / 2 && firstIndex > 0) - || firstIndex + size == array.length) { - // REVIEW: Why not evaluate (lastIndex==array.length) - // first to save the divide? + } else if (firstIndex + size == array.length + || (firstIndex > 0 && location < size / 2)) { System.arraycopy(array, firstIndex, array, --firstIndex, location); } else { @@ -180,8 +177,6 @@ public class ArrayList extends Abstra */ @Override public boolean addAll(int location, Collection collection) { - // REVIEW: Inconsistent exception case with - // add(location,object) method if (location < 0 || location > size) { throw new IndexOutOfBoundsException( // luni.0A=Index: {0}, Size: {1} @@ -198,30 +193,19 @@ public class ArrayList extends Abstra return false; } - // REVIEW: we use array.length - growSize in 3 places - - // precalculate it? if (location == 0) { growAtFront(growSize); firstIndex -= growSize; } else if (location == size) { - // REVIEW: Don't need the above check as it can be no - // other case. Make it just an else. if (firstIndex + size > array.length - growSize) { growAtEnd(growSize); } } else { // must be case: (0 < location && location < size) if (array.length - size < growSize) { - // REVIEW: why grow growSize? Does growForInsert() - // check we don't allocate too much? growForInsert(location, growSize); - } else if ((location < size / 2 && firstIndex > 0) - || firstIndex + size > array.length - growSize) { - // REVIEW: If condition above could be switched so - // divide is done 2nd, same as add() + } else if (firstIndex + size > array.length - growSize + || (firstIndex > 0 && location < size / 2)) { int newFirst = firstIndex - growSize; - // REVIEW: Have we optimised for the case where there - // is enough space at the end for growSize - // rather than doing 2 array copies? if (newFirst < 0) { int index = location + firstIndex; System.arraycopy(array, index, array, index - newFirst, @@ -259,8 +243,6 @@ public class ArrayList extends Abstra return false; } if (dumpArray.length > array.length - (firstIndex + size)) { - // REVIEW: why grow dumpArray.length rather than - // dumpArray.length - free space? growAtEnd(dumpArray.length); } System.arraycopy(dumpArray, 0, this.array, firstIndex + size, @@ -369,18 +351,12 @@ public class ArrayList extends Abstra } private void growAtEnd(int required) { - int lastIndex = firstIndex + size; - // REVIEW: Isn't this next condition just: - // required < array.length - size ? - if (firstIndex >= required - (array.length - lastIndex)) { - // REVIEW: Should use size! We don't seem to need newLast - // - just use size calculated above - int newLast = lastIndex - firstIndex; - // REVIEW: Why not just a !=0 check here? And size cannot - // be 0 unless required is 0, which does not happen - if (size > 0) { + if (array.length - size >= required) { + // REVIEW: as growAtEnd, why not move size == 0 out as + // special case + if (size != 0) { System.arraycopy(array, firstIndex, array, 0, size); - int start = newLast < firstIndex ? firstIndex : newLast; + int start = size < firstIndex ? firstIndex : size; // REVIEW: I think we null too much // array.length should be lastIndex ? Arrays.fill(array, start, array.length, null); @@ -397,9 +373,7 @@ public class ArrayList extends Abstra increment = 12; } E[] newArray = newElementArray(size + increment); - // REVIEW: Just check size !=0? same as earlier check - - // can be pulled out to earlier in the method? - if (size > 0) { + if (size != 0) { System.arraycopy(array, firstIndex, newArray, 0, size); firstIndex = 0; } @@ -412,10 +386,10 @@ public class ArrayList extends Abstra int newFirst = array.length - size; // REVIEW: as growAtEnd, why not move size == 0 out as // special case - if (size > 0) { + if (size != 0) { System.arraycopy(array, firstIndex, array, newFirst, size); - int length = firstIndex + size > newFirst ? newFirst - : firstIndex + size; + int lastIndex = firstIndex + size; + int length = lastIndex > newFirst ? newFirst : lastIndex; Arrays.fill(array, firstIndex, length, null); } firstIndex = newFirst; @@ -428,7 +402,7 @@ public class ArrayList extends Abstra increment = 12; } E[] newArray = newElementArray(size + increment); - if (size > 0) { + if (size != 0) { System.arraycopy(array, firstIndex, newArray, newArray.length - size, size); } @@ -525,16 +499,13 @@ public class ArrayList extends Abstra Integer.valueOf(location), Integer.valueOf(size))); } - if (location == size - 1) { + if (location == 0) { + result = array[firstIndex]; + array[firstIndex++] = null; + } else if (location == size - 1) { int lastIndex = firstIndex + size - 1; result = array[lastIndex]; array[lastIndex] = null; - } else if (location == 0) { - // REVIEW: this if test is simpler why isn't it first? - // (this moved up one place during the exception - // change but it still probably should move up one more) - result = array[firstIndex]; - array[firstIndex++] = null; } else { int elementIndex = firstIndex + location; result = array[elementIndex]; @@ -548,10 +519,10 @@ public class ArrayList extends Abstra array[firstIndex+size-1] = null; } } - // REVIEW: we can move this to the first if case since it - // can only occur when size==1 size--; + // REVIEW: we can move this to the first if case since it + // can only occur when size==1 if (size == 0) { firstIndex = 0; } Propchange: harmony/enhanced/java/branches/java6/drlvm/ ------------------------------------------------------------------------------ --- svn:mergeinfo (original) +++ svn:mergeinfo Thu Aug 12 14:06:31 2010 @@ -1,5 +1,5 @@ /harmony/enhanced/java/branches/mrh/drlvm:935751-941490 -/harmony/enhanced/java/trunk/drlvm:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731 +/harmony/enhanced/java/trunk/drlvm:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731,984779 /harmony/enhanced/trunk/drlvm:476395-929252 /harmony/enhanced/trunk/working_vm:476396-920147 /incubator/harmony/enhanced/trunk/drlvm:292550-476394 Propchange: harmony/enhanced/java/branches/java6/jdktools/ ------------------------------------------------------------------------------ --- svn:mergeinfo (original) +++ svn:mergeinfo Thu Aug 12 14:06:31 2010 @@ -1,4 +1,4 @@ -/harmony/enhanced/java/trunk/jdktools:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731 +/harmony/enhanced/java/trunk/jdktools:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731,984779 /harmony/enhanced/jdktools/trunk:630107-925933 /harmony/enhanced/trunk/jdktools:476395-929252 /harmony/enhanced/trunk/working_jdktools:476396-920147