RFR: 8332895: Support interpolation for backgrounds and borders [v3]

John Hendrikx jhendrikx at openjdk.org
Sat Aug 3 11:49:38 UTC 2024


On Sat, 3 Aug 2024 10:04:00 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

> Do you think it is preferrable to appeal to JIT magic instead of explicitly coding the behavior?

If the code leads to less questions and is more readable for it, and there is no reason to optimize in the first place (ie. you didn't see a problem), then yes, definitely.

Premature optimization only leads to questions by reviewers and future maintainers (they will wonder why the code was optimized, but probably assume there was a good reason for it).  Optimized code is harder to reason about and harder to refactor.  For example, reading your code I've been left wondering:

- Why is it significant to return a specific instance?  Are you using `==` to compare these in callers?  That's very counter to the nature of Java, and so deserves at a minimum a justification ("performance") and a warning ("careful, callers use `==`")
- The extra code paths for `RandomAccess` or not, in an attempt to avoid an iterator allocation (which may not even happen when inlined).  Is this even happening in practice (are there any non-`RandomAccesss` lists used?)  Is the performance gain worth the extra complexity?  Is it worth not having these functions inline into callers, as larger functions mean JIT won't inline them?
- Avoiding allocating small arrays; was this a performance problem that would justify writing a function that is now too large to inline? You're trading potential JIT inlining for an almost free heap allocation (Java allocation's are the equivalent of a pointer increment + background GC activity on likely **idling** CPU cores).

Would it not have been much better to write the code as simple as possible first, get it passed review well understood by everyone, then maybe, possibly get back to it later with an optimizing PR if there is a performance problem?  That last bit is entirely optional and may never happen if it turns out these few transition allocations are but a drop in the bucket of the overall CSS performance.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1522#discussion_r1702717395


More information about the openjfx-dev mailing list