RFR: 8377427: Reduce substring allocations in Color.web(String, double) [v7]
Andy Goryachev
angorya at openjdk.org
Tue Feb 17 19:11:28 UTC 2026
On Sat, 14 Feb 2026 14:04:08 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:
>> Color.web(string, double) parses a color string by creating substrings of the input. These string allocations can be removed.
>>
>> There are no new tests for the `Color` class, since the existing tests already cover all relevant code paths.
>
> Michael Strauß has updated the pull request incrementally with one additional commit since the last revision:
>
> review comment
modules/javafx.graphics/src/main/java/com/sun/javafx/css/parser/CssNumberParser.java line 34:
> 32: private CssNumberParser() {}
> 33:
> 34: // Up to 19 decimal digits fit in an unsigned 64-bit long
do we need to parse all 19 digits? An IEEE 64-bit double has 52 bits of mantissa, which translates into 2^53 = 9007199254740992 (length=16)
modules/javafx.graphics/src/main/java/com/sun/javafx/css/parser/CssNumberParser.java line 211:
> 209: *
> 210: * @see <a href="https://arxiv.org/pdf/2101.11408">Number Parsing at a Gigabyte per Second</a>
> 211: * @see <a href="https://arxiv.org/pdf/2212.06644">Fast Number Parsing Without Fallback</a>
First of all, I am not against writing more efficient or allocation-free code here.
I don't know whether CSS parsing is a frequent operation (it might be a frequent operation to _apply_ the styles once the stylesheet got parsed, but parsing itself should be run only when the stylesheet(s) have changed) - quite possibly this change will produce a smoother app, I just don't have the data.
My concern is that we are bringing some cutting edge code for `Color.web()` specifically, rather than rely on the JDK. Perhaps the optimization should be in the JDK in `Double.parseDouble()`?
On the other hand, we can be the trailblazers. Did you write this code or was it copied from some place? In other words, is it compatible with the license?
The second concern is testing. The new code is beyond what I consider a reasonable amount of time and effort that I can allocate to the code review. I mean, if we decide it is worth it, we can try, but I'd like @kevinrushforth to get involved.
The main claim is that the this code is indistinguishable from `Double.parseDouble()`, something that needs to be demonstrated. May be it'll be sufficient to add more patterns (with `Float`s, we could test the whole range, but it is unfeasible with 64-bit `Double`), especially near boundary points.
What do you think?
modules/javafx.graphics/src/main/java/com/sun/javafx/css/parser/CssNumberParser.java line 303:
> 301: * See "Table Generation Script" in "Number Parsing at a Gigabyte per Second" (Lemire, 2021), p. 32
> 302: *
> 303: * @see <a href="https://arxiv.org/pdf/2101.11408">Number Parsing at a Gigabyte per Second</a>
is it really worth to go to all that trouble?
Does the CssParser re-parses the stylesheet every time a Node changes, or does it parse the stylesheet(s) once and then works with the internal objects to apply the styles?
modules/javafx.graphics/src/test/java/test/com/sun/javafx/css/parser/CssNumberParserTest.java line 168:
> 166: public void roundingIsCorrectFor64BitSignificands() {
> 167: int seed = new Random().nextInt();
> 168: System.out.println("Testing CssNumberParserTest.roundingIsCorrectFor64BitSignificands with seed " + seed);
btw, the seed should be `long`
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2818425224
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2818584850
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2818630658
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2818509579
More information about the openjfx-dev
mailing list