Re: RFR 8234423 : Modifying ArrayList.subList().subList() resets modCount of subList

Thank you Roger for your review! Happy New Year to you too! :) With kind regards, Ivan On 1/9/20 10:36 AM, Roger Riggs wrote: Hi Ivan, Happy New Year! This change looks fine. Roger On 11/23/19 2:30 AM, Ivan Gerasimov wrote: Re-sending the request with the correct Subject line - the bug i

Re: RFR 8234423 : Modifying ArrayList.subList().subList() resets modCount of subList

Hi Ivan, Happy New Year! This change looks fine. Roger On 11/23/19 2:30 AM, Ivan Gerasimov wrote: Re-sending the request with the correct Subject line - the bug is not (only) about the iterator. On 11/20/19 10:11 PM, Ivan Gerasimov wrote: Hello! When a sublist of a sublist of an ArrayLi

Re: RFR 8234423 : Modifying ArrayList.subList().subList() resets modCount of subList

Gentle ping. Will someone please volunteer to review this fix of a regression? Thanks in advance! On 11/20/19 10:11 PM, Ivan Gerasimov wrote: Hello! When a sublist of a sublist of an ArrayList is created, its modCount is initialized from the ArrayList root, and not from its immediate pare

Re: RFR 8234423 : Modifying ArrayList.subList().subList() resets modCount of subList

Re-sending the request with the correct Subject line - the bug is not (only) about the iterator. On 11/20/19 10:11 PM, Ivan Gerasimov wrote: Hello! When a sublist of a sublist of an ArrayList is created, its modCount is initialized from the ArrayList root, and not from its immediate parent.

Re: RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

On 2018-05-15 03:45, Ivan Gerasimov wrote: let's move the line 586 final int otherModCount = other.modCount; to the beginning of equalsArrayList(ArrayList other), so it is initialized before other.size is read? Sure, let's also pick up on Martin's suggestion to rewrite the control

Re: RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

On 5/14/18 5:22 PM, Claes Redestad wrote: I was actually toying with and testing a change to this effect anyway, since it's a nice cleanup and might help the JIT somewhat: http://cr.openjdk.java.net/~redestad/8196340/open.02/ A note on correctness: none of the code in ArrayList that utili

Re: RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

On 2018-05-14 22:29, Ivan Gerasimov wrote: Thank you Claes! The mutator methods normally first update the modCount and then change the size of ArrayList. Then, in other methods the modCount is copied to a local variable first, and after that the size is copied. This is not true for equal

Re: RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

Thank you Claes! The mutator methods normally first update the modCount and then change the size of ArrayList. Then, in other methods the modCount is copied to a local variable first, and after that the size is copied. This is not true for equalsRange(List other, int from, int to) when it

Re: RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

On Mon, May 14, 2018 at 7:33 AM, Claes Redestad wrote: > >> I would prefer having only one comodification check for a bulk operation, >> but I understand that checking at each step is more compatible with the >> default implementation. >> >> 594 for (int i = 0; i < s; i++) { >> 595

Re: RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

Martin, On 2018-05-14 16:15, Martin Buchholz wrote: Claes, Looks good. thanks! I would move the size check up to the beginning of the method. 583         int expectedModCount = modCount;  584         int otherModCount = other.modCount;  585         int s = size;  586         if (s != othe

Re: RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

Claes, Looks good. I would move the size check up to the beginning of the method. 583 int expectedModCount = modCount; 584 int otherModCount = other.modCount; 585 int s = size; 586 if (s != other.size) { 587 return false; 588 } I would pr

Re: RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

Hi Ivan, right, checkForComodification() alone should be sufficient here. Updated in-place: http://cr.openjdk.java.net/~redestad/8196340/open.01/ Thanks! /Claes On 2018-05-12 03:38, Ivan Gerasimov wrote: Hi Claes! One thing I can't figure out is why both these two checks are necessary: 130

Re: RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

Hi Claes! One thing I can't figure out is why both these two checks are necessary: 1303 checkForComodification(); 1304 root.checkForComodification(expectedModCount); The former compares the current root.modCount with the one at the time this subList was created. The l

Re: RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

Anyhow, this is the current state of things: http://cr.openjdk.java.net/~redestad/8196340/open.01/ I think the remaining difference between ArrayList$SubList and COWAL is down to the comodification checks in the former eating up too much inlining budget, but haven't had any luck yet. /Claes

Re: RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

Good, this demonstrates the gain from specializing o.getClass() == ArrayList.class, which is most of the speed-up here as well as in my own micro: Patched: Method  Millis Ratio ArrayList equals    17 1.000 ArrayList$SubList equals   108 6.312 CopyOnWriteArray

Re: RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

Yet another microbenchmark coming your way. Embarrassing to have COWAL win all these so handily. --- IteratorMicroBenchmark.java 11 May 2018 18:19:10 - 1.45 +++ IteratorMicroBenchmark.java 11 May 2018 22:35:32 - 1.46 @@ -564,6 +564,12 @@ sum[0] = 0;

Re: RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

Right, and with the patch I've proposed ArrayList#hashCode is on par with current COWAL on my machine in your micro: $ ~/src/jdk/build/linux-x64/images/jdk/bin/java -cp ./open/test/jdk/java/util/Collection IteratorMicroBenchmark filter=ArrayList.*hashCode.* Method    Milli

Re: RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

Yet another way to iterate over a collection. Coming in via jsr166 soon. Seeing CopyOnWriteArrayList beat ArrayList decisively suggests that optimizing it is defintely worth doing. And of course the numbers below contain more low-hanging fruit if anyone still cares about the performance of Linke

Re: RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

ude a test to ArrayList when doing the instanceof ?? Rémi - Mail original - > De: "Claes Redestad" > À: "core-libs-dev" > Envoyé: Vendredi 11 Mai 2018 15:55:48 > Objet: RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList > and Ar

Re: RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

Hi, On 2018-05-11 19:02, Martin Buchholz wrote: Thanks Claes, This all looks correct, but I would - rename the ranged version of lastIndexOf lastIndexOfRange - introduce hashCodeRange and equalsRange - add "final" to all the Object[] es sure, - hesitate to optimize ArrayList.equals(Arra

Re: RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

Thanks Claes, This all looks correct, but I would - rename the ranged version of lastIndexOf lastIndexOfRange - introduce hashCodeRange and equalsRange - add "final" to all the Object[] es - hesitate to optimize ArrayList.equals(ArrayList). Do you have a particular use case in mind? At some

RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList

Hi, ArrayList doesn't override AbstracList#equals, and ArrayList$SubList doesn't override indexOf and equals. This provides specialized and more efficient implementations. Webrev: http://cr.openjdk.java.net/~redestad/8196340/open.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8196340 Test

Re: [PATCH] Inefficient ArrayList.subList().toArray()

On Mon, Jan 29, 2018 at 1:02 PM, Martin Buchholz wrote: > I'm going to expedite this a little since we are building further changes > on top. > > 8196207: Inefficient ArrayList.subList().toArray() > http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166- > integration/Ar

Re: [PATCH] Inefficient ArrayList.subList().toArray()

On Jan 29, 2018, at 4:00 PM, John Rose wrote: > > Then the various array-backed implementations > (and their sublists) would simply override their respective > stream views. BTW, this notion is more general than you might think. It applies to collections, like the internal spined buffer, which

Re: [PATCH] Inefficient ArrayList.subList().toArray()

Reviewed (officially). This is a good point-fix. — John P.S. I still think we have some tech. debt to discharge in finding a way to generically provide zero-copy access to array data, across interoperating collections APIs. If we got the deeper, more general answer right, we would be able to r

Re: [PATCH] Inefficient ArrayList.subList().toArray()

> On Jan 29, 2018, at 3:30 PM, Martin Buchholz wrote: > > > > On Mon, Jan 29, 2018 at 3:10 PM, Paul Sandoz > wrote: > > > > On Jan 29, 2018, at 1:02 PM, Martin Buchholz > > wrote: > > > > I'm going to expedite this a little since

Re: [PATCH] Inefficient ArrayList.subList().toArray()

On Mon, Jan 29, 2018 at 3:10 PM, Paul Sandoz wrote: > > > > On Jan 29, 2018, at 1:02 PM, Martin Buchholz > wrote: > > > > I'm going to expedite this a little since we are building further changes > > on top. > > > > RFR: jsr166 jdk integration 2018-02 > > http://cr.openjdk.java.net/~martin/webre

Re: [PATCH] Inefficient ArrayList.subList().toArray()

w.html > Looks ok, but i personally would not categorize the F/J changes as miscellaneous :-) > 8196207: Inefficient ArrayList.subList().toArray() > http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/ArrayList-subList-toArray/index.html > https://bugs.openjdk.java.net/browse/JDK-8196207 > Looks ok too. Paul.

Re: [PATCH] Inefficient ArrayList.subList().toArray()

I'm going to expedite this a little since we are building further changes on top. RFR: jsr166 jdk integration 2018-02 http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/overview.html 8196207: Inefficient ArrayList.subList().toArray() http://cr.openjdk.java.net/~martin/webrev

Re: [PATCH] Inefficient ArrayList.subList().toArray()

tin Buchholz wrote: > >> It's still possible to find simple performance improvements in classes >> like >> ArrayList. >> > > Indeed! > > It seems ArrayList.SubList delegates quite a few of its implementations > to AbstractList and AbstractCollection, wh

Re: [PATCH] Inefficient ArrayList.subList().toArray()

On 2018-01-26 07:20, Martin Buchholz wrote: It's still possible to find simple performance improvements in classes like ArrayList. Indeed! It seems ArrayList.SubList delegates quite a few of its implementations to AbstractList and AbstractCollection, which uses iterators for things that

Re: [PATCH] Inefficient ArrayList.subList().toArray()

Thanks for quick review! Indeed, there must be checkForComodification() at the beginning of each method, my bad. Btw, ArrayList produced by java.util.Arrays.asList() is also RandomAccess and array-based but has no custom implementation of subList(). Could we apply the same approach for java.ut

Re: [PATCH] Inefficient ArrayList.subList().toArray()

Yes. public Object[] toArray() { checkForComodification(); return Arrays.copyOfRange(root.elementData, offset, offset + size); } @SuppressWarnings("unchecked") public T[] toArray(T[] a) { checkForComodification(); if

Re: [PATCH] Inefficient ArrayList.subList().toArray()

On Jan 25, 2018, at 2:02 PM, Сергей Цыпанов wrote: > > +return (T[]) Arrays.copyOfRange(root.elementData, offset, > size, a.getClass()); Giving this a quick glance: I think you may want s/size/offset+size/. There should be calls to checkForComodification.

Re: [PATCH] Inefficient ArrayList.subList().toArray()

uge difference in absolute values. > > Instantiation of SubList costs in average only 9.591 ± 0.650 ns and is > independent on the size of its source so it couldn't be root cause. > > Here SubLists taken from Arra

Re: [PATCH] Inefficient ArrayList.subList().toArray()

same for both methods there's a huge difference in absolute values. > > Instantiation of SubList costs in average only 9.591 ± 0.650 ns and is > independent on the size of its source so it couldn't be root cause. > > Here SubLists taken from Arra

[PATCH] Inefficient ArrayList.subList().toArray()

of its source so it couldn't be root cause. Here SubLists taken from ArrayList calls AbstractCollection.toArray() method while implementing RandomAccess and being array-based by its nature. Array-based ArrayList provides efficient implementation of toArray(T[]) a

Re: RFR-8148748: ArrayList.subList().spliterator() is not late-binding

> On 4 Mar 2016, at 17:42, Tagir F. Valeev wrote: > > Hello! > > AIOOBE is possible for ArrayList itself as well. E.g.: > >ArrayList test = new ArrayList<>(Arrays.asList(1,2,3,4)); >Spliterator spltr = test.spliterator(); >spltr.tryAdvance(System.out::println); >test.clear(); >

Re: RFR-8148748: ArrayList.subList().spliterator() is not late-binding

Hello! AIOOBE is possible for ArrayList itself as well. E.g.: ArrayList test = new ArrayList<>(Arrays.asList(1,2,3,4)); Spliterator spltr = test.spliterator(); spltr.tryAdvance(System.out::println); test.clear(); test.trimToSize(); spltr.tryAdvance(System.out::println); R

Re: RFR-8148748: ArrayList.subList().spliterator() is not late-binding

> On 4 Mar 2016, at 15:38, Tagir F. Valeev wrote: > > Hello! > > Thank you for the review! > Thanks. I just realised there are some subtleties where if the top-level list is reduced in size the spliterator of a sublist may on traversal throw an ArrayIndexOutOfBoundsException rather than Co

Re: RFR-8148748: ArrayList.subList().spliterator() is not late-binding

Hello! Thank you for the review! Here's updated webrev: http://cr.openjdk.java.net/~tvaleev/webrev/8148748/r3/ PS> Looks good. I especially like: PS> 125 addCollection(l.andThen(list -> list.subList(0, list.size(; PS> Can you also update SpliteratorTraversingAndSplittingTest?

Re: RFR-8148748: ArrayList.subList().spliterator() is not late-binding

> On 4 Mar 2016, at 10:26, Tagir F. Valeev wrote: > > Hello! > >>> I'm just worrying a little that my changes might conflict with Ivan >>> Gerasimov's pending 8079136 issue, so probably it would be better to >>> wait till it's reviewed and pushed… > > Ivan said that 8079136 is stalled for a wh

Re: RFR-8148748: ArrayList.subList().spliterator() is not late-binding

Hello! >> I'm just worrying a little that my changes might conflict with Ivan >> Gerasimov's pending 8079136 issue, so probably it would be better to >> wait till it's reviewed and pushed… Ivan said that 8079136 is stalled for a while, so I decided to continue working on 8148748. Here's updated w

Re: RFR-8148748: ArrayList.subList().spliterator() is not late-binding

> On 8 Feb 2016, at 15:53, Tagir F. Valeev wrote: > > Hello! > > Sorry, I don't understand how passing AbstractList would help. Yeah, sorry ignore what i sad about AbstractList, was not thinking properly in my attempt to unify. > We > still need direct access to the elementData array of the

Re: RFR-8148748: ArrayList.subList().spliterator() is not late-binding

best regards, Tagir Valeev. PS> Hi Tagir, PS> It’s certainly easier to review if one avoids the temptation to PS> change other things at the same time :-) PS> There is a possible simplified approach to sharing one PS> Spliterator impl between ArrayList and ArrayList.SubList. The PS>

Re: RFR-8148748: ArrayList.subList().spliterator() is not late-binding

Hi Tagir, It’s certainly easier to review if one avoids the temptation to change other things at the same time :-) There is a possible simplified approach to sharing one Spliterator impl between ArrayList and ArrayList.SubList. The initial (origin, fence) for ArrayList can be (0, -1) and

Re: RFR-8148748: ArrayList.subList().spliterator() is not late-binding

Hello! Thank you for clarification. As far as I know currently JIT-compiled null checks are almost always implicit (via trapped page-fault). Well, ok, I'm not a Hotspot expert (probably for C1 this still matters). I will keep this code as is. Webrev was not changed: http://cr.openjdk.java.net/~tv

Re: RFR-8148748: ArrayList.subList().spliterator() is not late-binding

On Wed, Feb 3, 2016 at 4:20 AM, Tagir F. Valeev wrote: > Some more thoughts about forEachRemaining: > > To me it seems that > if ((a = lst.elementData) != null) > is a redundant check as well as in current ArrayList implementation > elementData is never null. So it can be replaced with simple > a

RFR-8148748: ArrayList.subList().spliterator() is not late-binding

such PS> * a fashion that iterations in progress may yield incorrect results.) PS> Paul. >> On 29 Jan 2016, at 05:32, Tagir F. Valeev wrote: >> >> Hello! >> >> When reviewing 8079136 I noticed that >> ArrayList.subList().spliterator()

Re: ArrayList.subList().spliterator() is not late-binding

. > On 29 Jan 2016, at 05:32, Tagir F. Valeev wrote: > > Hello! > > When reviewing 8079136 I noticed that > ArrayList.subList().spliterator() is not late-binding: > > List list = new ArrayList<>(Arrays.asList(1,2,3,4)).subList(0, 3); > Stream s = list.strea

ArrayList.subList().spliterator() is not late-binding

Hello! When reviewing 8079136 I noticed that ArrayList.subList().spliterator() is not late-binding: List list = new ArrayList<>(Arrays.asList(1,2,3,4)).subList(0, 3); Stream s = list.stream(); list.add(5); s.findFirst(); --> Exception in thread "main" java.util.ConcurrentMo

Re: ArrayList.subList

On 07/29/11 06:34, Doug Lea wrote: As a bandaid, a faster but still compatible path for simple get/set operations on ArrayList.subList could be put in at the expense of adding a a few extra fields. I should have checked before posting -- exactly this bandaid already exists in current version

Re: ArrayList.subList

g list", which would otherwise naturally refer to the ArrayList. This would be challenging to fix in ArrayList while retaining compatibility, even though it is sensibly ignored in other List implementations. As a bandaid, a faster but still compatible path for simple get/set operations on Array

Re: ArrayList.subList

fashion that iterations in progress may yield incorrect results.) On Jul 28 2011, at 13:40 , Doug Lea wrote: While puzzling over why one of the demo videos on ForkJoin out there required surprisingly high sequential thresholds, I noticed that ArrayList.subList creates lists with per-access o

Re: ArrayList.subList

s, I > noticed that ArrayList.subList creates lists with per-access > overhead proportional to the sublist depth. Which is > not at all conducive to recursive use. Does anyone > know any reason why this was done? (No one who I > thought would know the answer does.) It is easy to kee

ArrayList.subList

While puzzling over why one of the demo videos on ForkJoin out there required surprisingly high sequential thresholds, I noticed that ArrayList.subList creates lists with per-access overhead proportional to the sublist depth. Which is not at all conducive to recursive use. Does anyone know any