RFR: JDK-8322964 Optimize performance of CSS selector matching [v5]

[Andy Goryachev]

On Sat, 9 Mar 2024 05:26:33 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> modules/javafx.graphics/src/main/java/com/sun/javafx/css/FixedCapacitySet.java line 135:
>> 
>>> 133:     protected final void ensureNotFrozen() {
>>> 134:         if (frozen) {
>>> 135:             throw new UnsupportedOperationException();
>> 
>> should we explain why?  "the set is frozen" or something like that.
>
> I'm not against adding an explanation, but it's not needed as this is part of the `Set` contract (sets that can't be modified are specified to throw `UnsupportedOperationException`), for example for `add` in `Set`:
> 
>      * @throws UnsupportedOperationException if the {@code add} operation
>      *         is not supported by this set
> 
> The concept of "freezing" the set is not visible to any users, it's just there to avoid having to make a final copy of the set. It's modifiable by the code that created it, but if they freeze it before exposing it to the outside world, then for all intents and purposes it is a read only set for anyone else.

But it is somewhat visible: **public** `void freeze();` (although it cannot be invoked directly).

Edit: While I accept your reasoning that the functionality is effectively hidden from the user, I still think that the exception should always explain "why", if only to reduce the cognitive load.  Think of all the countless man hours saved collectively over the years.  Yes, the answer can be found by googling, stackoverflowing, and reading RTFM, but if the answer is right there it is so much better.  Wouldn't you agree?

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1316#discussion_r1519862160