UPDATE (2024-06-20): while intended to just be an internal refactor, the changes in this PR do appear to fix the source of errors being thrown for some users on Windows 10. The cause of these errors is still unknown.
I wanted to do this while the code was still fresh in my mind. There are no expected functional changes here, just code simplification.
Currently, previousRenderTimes is used for easier lookup into event-timing-entry groups in pendingEntriesGroupMap and for quick event grouping by render time, but it's also not quite the same set of render times in pendingEntriesGroupMap (the times and groups grow together as events come in, but the map actually contains all the events referenceable by previousRenderTimesand groups with events from longestInteractionList). The results in more bookkeeping and (speaking as someone recently getting their head around this code :) it can make some of this logic harder to follow because effects aren't always collocated.
This PR proposes getting rid of previousRenderTimes so that there are only two main data structures, an array tracking LoAFs and an array tracking groups of event timing entries. I think this makes it a little simpler to understand how events are grouped as they come in and which event groups are being preserved during cleanup, without a runtime or clarity tradeoff elsewhere.
latestProcessingEnd and the entry WeakMap are still here as secondary data sources because they're convenient ways to skip extra work and they're both only ever updated in a single place (the WeakMap now maps directly to entry groups, though).
UPDATE (2024-06-20): while intended to just be an internal refactor, the changes in this PR do appear to fix the source of errors being thrown for some users on Windows 10. The cause of these errors is still unknown.
I wanted to do this while the code was still fresh in my mind. There are no expected functional changes here, just code simplification.
Currently,
previousRenderTimes
is used for easier lookup into event-timing-entry groups inpendingEntriesGroupMap
and for quick event grouping by render time, but it's also not quite the same set of render times inpendingEntriesGroupMap
(the times and groups grow together as events come in, but the map actually contains all the events referenceable bypreviousRenderTimes
and groups with events fromlongestInteractionList
). The results in more bookkeeping and (speaking as someone recently getting their head around this code :) it can make some of this logic harder to follow because effects aren't always collocated.This PR proposes getting rid of
previousRenderTimes
so that there are only two main data structures, an array tracking LoAFs and an array tracking groups of event timing entries. I think this makes it a little simpler to understand how events are grouped as they come in and which event groups are being preserved during cleanup, without a runtime or clarity tradeoff elsewhere.latestProcessingEnd
and the entry WeakMap are still here as secondary data sources because they're convenient ways to skip extra work and they're both only ever updated in a single place (the WeakMap now maps directly to entry groups, though).