Closed JobLeonard closed 4 years ago
Really nice suggestion! I got curious so I made a quick benchmark running on 1000+ render tests in Mapbox GL JS, commenting out the "identical images" fast path — this change doesn't seem to have any noticeable performance gain, so probably not worth sacrificing code clarity for. The low impact is probably due to the fact that for every full-color colorDelta
call, we do 16 brightness-only ones, which benefit from the micro-optimization less.
Also an off-topic fun fact from the bench: it takes 18 times as long to png-decode the fixtures compared to pixelmatch running time. 😅
The low impact is probably due to the fact that for every full-color colorDelta call, we do 16 brightness-only ones, which benefit from the micro-optimization less.
I mean, that's also a reduction from six to three multiplications, so percentage-wise the difference is even bigger ;)
Also an off-topic fun fact from the bench: it takes 18 times as long to png-decode the fixtures compared to pixelmatch running time. :sweat_smile:
Ouch! Just shows how important it is do benchmark properly ;)
Anyway, thanks for checking! It was a fun thought-exercise at least :)
Recently I adapted the
colorDelta
function from the code in this repo in one of my Observable notebooks, and more or less on autopilot inlinedrgb2y
,rgb2i
, andrgb2q
:This removes six function calls, six multiplications and three additions. Since this is a hot function I'm sure that the function calls are probably inlined anyway, but the JIT compiler can't perform the other optimization for us ;). Since
colorDelta
is the heart of this algorithm I wonder if that small change adds up or not.Also, now that I'm writing this out it got me thinking: barring rounding errors the following should be equivalent:
The latter form would save another three multiplications, for a total of nine (going from 21 down to 12).
So I haven't benchmarked any of this, so I don't know the actual impact on performance it has, but technically it should be less work for the CPU at least. I guess we should also check if the results are the same - I did add the change described in this comment to my notebook and it seems to work fine, but that's not a pixel-to-pixel comparison ;)
The actual code would probably look something like: