On Tue, 17 Feb 2026 18:56:34 GMT, Andy Goryachev <[email protected]> wrote:
>> 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 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? The code is not copied, it's an implementation of the algorithm presented in the referenced paper. The algorithm is what I would call state of the art, it's used in several standard libraries: 1. [Go since v1.16](https://go.dev/doc/go1.16#strconvpkgstrconv) 2. [Rust since v1.55](https://blog.rust-lang.org/2021/09/09/Rust-1.55.0/#faster-more-correct-float-parsing) 3. [GNU libstdc++](https://gcc.gnu.org/pipermail/libstdc%2B%2B-cvs/2022q1/037428.html) 4. [LLVM libc](https://libc.llvm.org/configure.html) has a toggle for it The problem with `Double.parseDouble()` is not that it's slow, or mainly that it requires substring allocation (although that's unfortunate), it's that it parses a Java-specific number grammar, where it should be parsing CSS numbers. The purpose of code review also can't be to demonstrate that Lemire's algorithm is indistinguishable from the JDK's algorithm. How would you even do that? I certainly can't prove that mathematically, and enumerating all doubles is impossible. If this algorithm produces correctly-rounded doubles, then it must be equivalent to the JDK algorithm. As I said, the algorithm is state of the art, and people more knowledgeable than I have obviously concluded that it's correct, so I think we can leave it at that. What remains is to verify that the _implementation_ of the algorithm is correct, and from all the testing I haven't found a single example that would be different from the JDK algorithm. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2069#discussion_r2818763034
