RFR: 8185831: Pseudo selectors do not appear to work in Node.lookupAll() [v2]

[Andy Goryachev]

On Thu, 19 Oct 2023 15:04:59 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> I must say I'm again baffled at how this is implemented.  
>> 
>> Red flag one: it uses a `LinkedList` which are known to be rarely the right choice, and I doubt this case is any different.  
>> 
>> Red flag two: a `List` is converted to a `Set`; being backed by a `LinkedList` means set type operations will be terribly slow if that list has any kind of size.  
>> 
>> Red flag three: The choice to return a `Set` in the public API seems to be only motivated to indicate there won't be any duplicates, which is a terrible reason.
>> 
>> Red flag four: `UnmodifiableListSet` talks about insertion speed and hashing overhead, yet uses a `LinkedList` which are slower than `ArrayList` when it comes to insertion speed (not to mention that they require more object allocations and more memory(!)).
>
> You are right, @hjohn - LinkedList seems like a bad choice!  It should be HashSet from the very beginning, shouldn't it?
> 
> But Set<Node> as a return value for lookups is correct, I think.
> 
> We could (should?) fix it in a separate PR.

created * [JDK-8318842](https://bugs.openjdk.org/browse/JDK-8318842) Node.lookupAll to use Set instead of List (Enhancement)

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1245#discussion_r1373382130