List#toArray question
|
View:
New views
12 Messages
—
Rating Filter:
Alert me
|
 |
List#toArray question
by Dimitris Andreou
::
Rate this Message:
I'm reading the 5th chapther of the 2nd version of Effective Java
(available here: http://www.infoq.com/resource/articles/bloch-effective-java-2e/en/resources/Bloch_Ch05.pdf) and I'm confused at page 121. It talks about reducing a list with a function that is applied to its elementsm and gives this code: // Reduction without generics or concurrency flaw static Object reduce(List list, Function f, Object initVal) { Object[] snapshot = list.toArray(); // Locks list internally Object result = initVal; for (Object o : list) result = f.apply(result, o); return result; } (I hope the formatting is preserved). What is this "Locks list internally" comment? I can't find any such locking (looking Sun's Java 6 implementation), neither in AbstractList and above, nor say in ArrayList (which delegates to Arrays#copyOf(Object[], int) - could that imply that System#arraycopy locks its arrays? But then still, that affects only ArrayLists...). And I would be confused if there was any. Did I miss anything obvious? Dimitris Andreou _______________________________________________ Concurrency-interest mailing list Concurrency-interest@... http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest |
|
|
Re: List#toArray question
by Holger Hoffstätte-2
::
Rate this Message:
Dimitris Andreou wrote:
> // Reduction without generics or concurrency flaw > static Object reduce(List list, Function f, Object initVal) { > Object[] snapshot = list.toArray(); // Locks list internally > Object result = initVal; > for (Object o : list) > result = f.apply(result, o); > return result; > } > > (I hope the formatting is preserved). > What is this "Locks list internally" comment? I can't find any such Seems like a bug (how ironic). The loop should iterate over the snapshot[], not the list - but the compiler did not complain because foreach handles both arrays and collections. Any decent IDE should warn that snapshot is never read. A great showcase for the dangers of ignoring compiler warnings.. Or, as a wise man once said: "-Wall -Werror..you may hate it now, but you'll learn to love it later" :-) -h _______________________________________________ Concurrency-interest mailing list Concurrency-interest@... http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest |
|
|
Re: List#toArray question
by Dimitris Andreou
::
Rate this Message:
O/H Holger Hoffstätte έγραψε:
> Dimitris Andreou wrote: >> // Reduction without generics or concurrency flaw >> static Object reduce(List list, Function f, Object initVal) { >> Object[] snapshot = list.toArray(); // Locks list internally >> Object result = initVal; >> for (Object o : list) >> result = f.apply(result, o); >> return result; >> } >> >> (I hope the formatting is preserved). >> What is this "Locks list internally" comment? I can't find any such > > Seems like a bug (how ironic). The loop should iterate over the > snapshot[], not the list - but the compiler did not complain because > foreach handles both arrays and collections. > Any decent IDE should warn that snapshot is never read. A great > showcase for the dangers of ignoring compiler warnings.. > > Or, as a wise man once said: > "-Wall -Werror..you may hate it now, but you'll learn to love it > later" :-) > > -h > Heh, I didn't notice that at all, it definitely looks like a typo. But my question regarded the first line of the method (and specifically, Bloch's comment), not the iteration below. Dimitris _______________________________________________ Concurrency-interest mailing list Concurrency-interest@... http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest |
|
|
Re: List#toArray question
by Holger Hoffstätte-2
::
Rate this Message:
Dimitris Andreou wrote:
> O/H Holger Hoffstätte έγραψε: >> Dimitris Andreou wrote: >>> // Reduction without generics or concurrency flaw >>> static Object reduce(List list, Function f, Object initVal) { >>> Object[] snapshot = list.toArray(); // Locks list internally >>> Object result = initVal; >>> for (Object o : list) >>> result = f.apply(result, o); >>> return result; >>> } >>> >>> (I hope the formatting is preserved). >>> What is this "Locks list internally" comment? I can't find any such >> >> Seems like a bug (how ironic). The loop should iterate over the >> snapshot[], not the list - but the compiler did not complain because >> foreach handles both arrays and collections. >> Any decent IDE should warn that snapshot is never read. A great >> showcase for the dangers of ignoring compiler warnings.. >> >> Or, as a wise man once said: >> "-Wall -Werror..you may hate it now, but you'll learn to love it >> later" :-) >> >> -h >> > > Heh, I didn't notice that at all, it definitely looks like a typo. But > my question regarded the first line of the method (and specifically, > Bloch's comment), not the iteration below. That's the point: when you iterate over the method-local snapshot, the original list is not touched anymore and therefore the iteration is "safe" from side effects (strictly speaking the computation as a whole is not). Other than that I don't think there is any other "locking" going on, at least not in the j.u.c.Lock sense of the word. -h _______________________________________________ Concurrency-interest mailing list Concurrency-interest@... http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest |
|
|
Re: List#toArray question
by Joe Bowbeer
::
Rate this Message:
On Tue, May 20, 2008 at 8:42 AM, Dimitris Andreou wrote:
I'm reading the 5th chapther of the 2nd version of Effective Java I see what you mean now that you point it out. An alternative you didn't mention is replacing "List" with "Vector", whose methods are locked internally. -- Joe Bowbeer _______________________________________________ Concurrency-interest mailing list Concurrency-interest@... http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest |
|
|
Re: List#toArray question
by Nathan Bronson
::
Rate this Message:
The snapshot code is definitely not safe for an arbitrary List, since
any implementation that inherits AbstractCollection.toArray() will just perform an iteration internally (with no lock). Perhaps this code originally used a Vector, for which toArray() is synchronized? Nathan On Tue, 2008-05-20 at 19:45 +0300, Dimitris Andreou wrote: > O/H Holger Hoffstätte έγραψε: > > Dimitris Andreou wrote: > >> // Reduction without generics or concurrency flaw > >> static Object reduce(List list, Function f, Object initVal) { > >> Object[] snapshot = list.toArray(); // Locks list internally > >> Object result = initVal; > >> for (Object o : list) > >> result = f.apply(result, o); > >> return result; > >> } > >> > >> (I hope the formatting is preserved). > >> What is this "Locks list internally" comment? I can't find any such > > > > Seems like a bug (how ironic). The loop should iterate over the > > snapshot[], not the list - but the compiler did not complain because > > foreach handles both arrays and collections. > > Any decent IDE should warn that snapshot is never read. A great > > showcase for the dangers of ignoring compiler warnings.. > > > > Or, as a wise man once said: > > "-Wall -Werror..you may hate it now, but you'll learn to love it > > later" :-) > > > > -h > > > > Heh, I didn't notice that at all, it definitely looks like a typo. But > my question regarded the first line of the method (and specifically, > Bloch's comment), not the iteration below. > > Dimitris > _______________________________________________ > Concurrency-interest mailing list > Concurrency-interest@... > http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest _______________________________________________ Concurrency-interest mailing list Concurrency-interest@... http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest |
|
|
Re: List#toArray question
by Joe Bowbeer
::
Rate this Message:
Ah. Here's what you're missing:
...suppose you have a synchronized list (of the sort returned by Collections.synchronizedList) ... On Tue, May 20, 2008 at 9:58 AM, Joe Bowbeer wrote:
_______________________________________________ Concurrency-interest mailing list Concurrency-interest@... http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest |
|
|
Re: List#toArray question
by Joshua Bloch-2
::
Rate this Message:
Hi. I was just about to say what Joe Bowbeer just said. The first sentence of the first paragraph on the page says:
For example, suppose you have a synchronized list (of the sort returned by Collections.synchronizedList) I hope people don't get confused by this:(
Keep in mind that the code above it ("// Reduction without generics, and with concurrency flaw!") would be garbage if the list weren't synchronized: it depends on the synchronized block to exclude concurrent updates. (It is intentionally broken, in that it calls an alien method inside the synchronized block, which is what is fixed in the example below.)
Regards,
Josh
On Tue, May 20, 2008 at 10:12 AM, Joe Bowbeer <joe.bowbeer@...> wrote: Ah. Here's what you're missing: _______________________________________________ Concurrency-interest mailing list Concurrency-interest@... http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest |
|
|
Re: List#toArray question
by Dimitris Andreou
::
Rate this Message:
That's it, thanks! (That was too far to the top :) )
O/H Joe Bowbeer έγραψε: > Ah. Here's what you're missing: > > ...suppose you have a synchronized list (of the sort returned by > Collections.synchronizedList) ... > > > On Tue, May 20, 2008 at 9:58 AM, Joe Bowbeer wrote: > > On Tue, May 20, 2008 at 8:42 AM, Dimitris Andreou wrote: > > I'm reading the 5th chapther of the 2nd version of Effective Java > (available here: > http://www.infoq.com/resource/articles/bloch-effective-java-2e/en/resources/Bloch_Ch05.pdf) > > and I'm confused at page 121. > > It talks about reducing a list with a function that is applied > to its > elements and gives this code: > > > // Reduction without generics or concurrency flaw > static Object reduce(List list, Function f, Object initVal) { > Object[] snapshot = list.toArray(); // Locks list internally > Object result = initVal; > for (Object o : list) > result = f.apply(result, o); > return result; > } > > (I hope the formatting is preserved). > What is this "Locks list internally" comment? I can't find any > such > locking (looking Sun's Java 6 implementation), neither in > AbstractList > and above, nor say in ArrayList (which delegates to > Arrays#copyOf(Object[], int) - could that imply that > System#arraycopy > locks its arrays? But then still, that affects only > ArrayLists...). And > I would be confused if there was any. Did I miss anything obvious? > > Dimitris Andreou > > > > I see what you mean now that you point it out. > > An alternative you didn't mention is replacing "List" with > "Vector", whose methods are locked internally. > > -- > Joe Bowbeer > > > ------------------------------------------------------------------------ > > _______________________________________________ > Concurrency-interest mailing list > Concurrency-interest@... > http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest > _______________________________________________ Concurrency-interest mailing list Concurrency-interest@... http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest |
|
|
Re: List#toArray question
by Joshua Bloch-2
::
Rate this Message:
Dimitris,
Thanks for telling me. If too many people get confused by this, I'll have to figure out some way to clarify it.
Josh
On Tue, May 20, 2008 at 10:59 AM, Dimitris Andreou <jim.andreou@...> wrote: That's it, thanks! (That was too far to the top :) ) _______________________________________________ Concurrency-interest mailing list Concurrency-interest@... http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest |
|
|
Re: List#toArray question
by Joe Bowbeer
::
Rate this Message:
Josh,
As Holger points out, there *is* a problem with this intermediate code: Prior to release 1.5, the natural way to do this would have been using List's toArray method (which locks the list internally): // Reduction without generics or concurrency flaw static Object reduce(List list, Function f, Object initVal) { Object[] snapshot = list.toArray(); // Locks list internally Object result = initVal; for (Object o : list) result = f.apply(result, o); return result; } It should be iterating over *snapshot* not list, and using an old-style for loop, because foreach didn't exist prior to Java 5. --Joe 2008/5/20 Joshua Bloch:
_______________________________________________ Concurrency-interest mailing list Concurrency-interest@... http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest |
|
|
Re: List#toArray question
by Joshua Bloch-2
::
Rate this Message:
Joe,
Yes. I have publically 'fessed up to this one: http://www.infoq.com/articles/bloch-effective-java-2e . I'm really unhappy with myself: I found this before the book went to pressed, fixed it in the code bundle, but somehow I don't fix it in the book. Sigh...
Josh
P.S. I'll send a fixed PDF to infoQ today.
2008/5/20 Joe Bowbeer <joe.bowbeer@...>: Josh, _______________________________________________ Concurrency-interest mailing list Concurrency-interest@... http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest |
| Free Forum Powered by Nabble | Forum Help |