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

John Hendrikx jhendrikx at openjdk.org
Sat Aug 3 14:11:39 UTC 2024


On Sat, 3 Aug 2024 11:43:12 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> There's no performance problem in the sense that my test application runs slowly in any noticeble way. It's one more piece of trying to minimize the allocation of objects in the very common case where nothing has changed.
>> 
>> Do you think it is preferrable to appeal to JIT magic instead of explicitly coding the behavior? With your other suggestion of using `switch`, the code looks much cleaner already.
>
>> 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.

Sorry if this came across as a bit negative, it's not intended in that way.  I just think optimizing too early has far more drawbacks than it has benefits. In part this is also a reflection on a lot of FX code; some code is clearly optimized from a C/C++ developer perspective which not only translates poorly to Java, but also makes the code "too dangerous" to modify.

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

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


More information about the openjfx-dev mailing list