V8 deopts diff

[marklundin]

Useful to know but It seems V8 tries to optimise the diff function, but bails out with the 'Optimized too many times' reason. You can test this by running the profiler in Chrome and checking the Timeline.

Not sure what the cause of this could be, but I might do a little more digging. Would be great to open this up to the optimising compiler :)

[developit]

@marklundin Is there a specific demo or repro you are seeing the de-opt in? Diff accepts a lot of different inputs, which means it tends to ride on the limits of V8's IC size, so I imagine there are some cases where that function switches from being polymorphic to megamorphic, depending on how Preact is used. I'm not seeing a de-opt in dbmonster, but perhaps that demo is not stress-testing varied diff() inputs enough.

Interesting little factoid while you're digging around - one of the reasons diff() is relatively small and most of the diffing is handled by diffNode() / diffAttributes() is because it's difficult to control inputs to diff() itself - it accepts a DOM node as one of its arguments, and DOM nodes have wildly varying shape (unfortunate really). My hope in separating the two was that it would be possible to only pass on things like .childNodes to diffNode() from diff(), which always has the same type.

Thanks for the extra eyes on performance, by the way! :)

[marklundin]

Great answer, and you're right about dbmonster. I'm not seeing any deopts either. I suspect thats because as you said the inputs are near identical across calls. I'll try and run it through IRHydra to see if it throws up anything.

[developit]

IRHydra is awesome, been using it to optimize for a few months. There are 2 or 3 functions in Preact that still hard de-opt, it'd be great to have another set of eyes looking for reasons.

[developit]

@marklundin Big thanks for pointing this out - I tried to strip back the use of deeply nested (dom) objects in some of the diff methods in 32c8aca and I think it should address that de-opt.

What should we do with this issue? I like to have performance visibility, but I don't have anything on hand that I can try to further optimize diff(). Maybe set up a performance backlog of some kind? I would love to have more performance metrics/tests in the suite - there are only two right now, but they have been immensely useful for catching performance regressions.

[marklundin]

This looks great, V8 is no longer bailing out of the diff. I think it's safe to close this off but agree it would be great to keep an eye on these types of things

[developit]

Great! 🎉