perf: Skip lifecycle processing work when the queue is empty - #3978
perf: Skip lifecycle processing work when the queue is empty#3978spydon wants to merge 2 commits into
Conversation
| final child = event.child!; | ||
| final parent = event.parent!; | ||
| if (_blocked.contains(child) || _blocked.contains(parent)) { | ||
| if (_blocked.isNotEmpty && |
There was a problem hiding this comment.
is the isNotEmpty saving any time here, the contains should be O(0) in that case?
There was a problem hiding this comment.
It saves a little bit since it has to calculate the identityHashCode in contains, meanwhile isNotEmpty is virtually free.
There was a problem hiding this comment.
is this because _blocked is almost always empty? that doesn't appear to necessarily be the case here.
I honestly don't see how a hash check could be any sort of bottleneck, in fact if it were wouldn't the set contains implementation do this? according to clanker the dart identityHashCode implementation is cached per object and saved on the 64-bit pointer header.
I am ok with keeping if we have evidence but I am in favour of having commented out justifications for non-obvious micro-optimizations such as this. as is this looks like a silly mistake. a simple one line comment justifying the cost and why this is such a hot path would make it more clear.
thinking more broadly I am now wondering if this repeat with blocked thing is not ideal to begin with, there might be a way to sort the events in a deterministically correct order, but that is out of scope, just a thought.
luanpotter
left a comment
luanpotter
left a comment
There was a problem hiding this comment.
LGTM, I am not convinced of the isNotEmpty skip but leave it to you
2 participants
Description
processLifecycleEventsnow returns immediately when the queue is empty instead of allocating a set and a closure on every tick, the reorder-parents set is only allocated when a priority change is actually queued, and the blocked-set hash lookups are skipped while the set is empty (the common single-pass case).Extracted from #3960 so the data-structure change there stands alone (as requested in this comment). Behavior is unchanged; this only removes per-tick allocations and lookups from the game loop.
Checklist
docsand added dartdoc comments with///.examplesordocs.Breaking Change?
Related Issues
Relates to #3957