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