RFR: 8377427: Reduce substring allocations in Color.web(String, double) [v4]
John Hendrikx
jhendrikx at openjdk.org
Sat Feb 14 00:51:05 UTC 2026
On Fri, 13 Feb 2026 20:40:33 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 comments
Marked as reviewed by jhendrikx (Reviewer).
modules/javafx.graphics/src/main/java/com/sun/javafx/css/parser/CssNumberParser.java line 33:
> 31:
> 32: // Up to 19 decimal digits fit in a signed 64-bit long
> 33: private static final int MAX_SIG_DIGITS = 19;
Not sure if this should matter, but even though that's true for a `long`, a `double` holds about 4-5 less significant digits, so you could stop earlier I suppose.
modules/javafx.graphics/src/main/java/javafx/scene/paint/Color.java line 404:
> 402: int offset = 0;
> 403:
> 404: if (colorString.regionMatches(true, 0, "#", 0, 1)) {
You sure that's better than `colorString.charAt(0) == '#'` ? Same for the other ones that don't need case insensitive matching.
modules/javafx.graphics/src/main/java/javafx/scene/paint/Color.java line 447:
> 445: int b = Integer.parseInt(colorString, offset + 2, offset + 3, 16);
> 446: int a = Integer.parseInt(colorString, offset + 3, offset + 4, 16);
> 447: yield Color.color(r / 15.0, g / 15.0, b / 15.0, opacity * a / 15.0);
I suppose that works, could also consider `(r << 4) + r` and call `Color.rgb`; it may not matter if things are stored internally as doubles.
modules/javafx.graphics/src/test/java/test/com/sun/javafx/css/parser/CssNumberParserTest.java line 52:
> 50: assertEquals(-0.5, parseDouble("-.5"), 0.0);
> 51: assertEquals(12.34, parseDouble("12.34"), 0.0);
> 52: assertEquals(0.00123, parseDouble("0.00123"), 1e-18);
It needs the 1e-18 here?
-------------
PR Review: https://git.openjdk.org/jfx/pull/2069#pullrequestreview-3800316830
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2806667490
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2806675101
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2806691789
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2806700044
More information about the openjfx-dev
mailing list