RFR: 8377427: Reduce substring allocations in Color.web(String, double) [v16]
John Hendrikx
jhendrikx at openjdk.org
Wed Feb 25 11:29:55 UTC 2026
On Mon, 23 Feb 2026 21:33:21 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.
>>
>> CSS numbers are parsed by `CssNumberParser`, which parses CSS-compliant numbers using Lemire's algorithm.
>>
>> 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
Marked as reviewed by jhendrikx (Reviewer).
modules/javafx.graphics/src/main/java/com/sun/javafx/css/parser/CssNumberParser.java line 182:
> 180: }
> 181:
> 182: // Use Lemire's algorithm if possible, otherwise fall back to Double.parseDouble()
FWIW, I think that this issue has become a bit ridiculous, but kudo's for sticking with it. I personally think the fallback is completely unnecessary, since it won't matter when the difference is in the 20th decimal place orso. But I'm in favor of not reopening this further.
modules/javafx.graphics/src/main/java/com/sun/javafx/css/parser/CssNumberParser.java line 236:
> 234: // Subnormals (lines 11-15)
> 235: if (p <= -1022) {
> 236: m >>>= -1022 - p;
Did a `p + 1` get dropped here?
modules/javafx.graphics/src/test/java/test/com/sun/javafx/css/parser/CssNumberParserTest.java line 215:
> 213: public void roundingIsCorrectForSubnormals() {
> 214: long seed = new Random().nextLong();
> 215: System.out.println("Testing CssNumberParserTest.roundingIsCorrectForSubnormals with seed " + seed);
I'll object to this and the entire reasoning that led to this. JUnit tests should be reproducable, not potentially fail arbitrarily in the run two days after a release was done. If we're not convinced that the tests are sufficient as is, then this certainly won't convince me either (quite the opposite in fact). In other words, this adds zero value.
-------------
PR Review: https://git.openjdk.org/jfx/pull/2069#pullrequestreview-3850898435
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2849890536
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2849911136
PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2849956701
More information about the openjfx-dev
mailing list