react-native icon indicating copy to clipboard operation
react-native copied to clipboard

refactor: use map for user timing API for better performance (fixes #45122)

Open robik opened this issue 1 year ago • 3 comments
trafficstars

Summary:

Fix #45122.

performance.mark is currently O(1) but performance.clearMark is O(n) (being n the number of entries in the buffer), which makes this operation very slow.

Changes overview

  • Created new PerformanceEntryBuffer abstraction with the following subtypes, that differ on how entries are stored:
    • PerformanceEntryCircularBuffer - stores them in a ring buffer that was already implemented, removed key lookup cache (BoundedConsumableBuffer)
    • PerformanceEntryKeyedBuffer - stores them in a unordered_map, allowing for faster retrieval by type
    • PerformanceEntryLinearBuffer - a simple infinite buffer based on std::vector, currently used in a PerformanceObserver
  • Created PerformanceObserver abstraction on native side.
  • Created PerformanceObserverRegistry that collects active observers and forwards entries to observers that should retrieve them
  • Moved some method implementations to .cpp files to reduce potential compilation time slowdown. As the PerformanceEntryReporter can be included from anywhere in the code, it will be beneficial to make header files as light as possible.
  • Add comments to methods that note which standard is the method from/for.
  • Added some [[nodiscard]] attributes
  • Since the logic of routing entries to observers is moved to native side, JS side of the code got much simplified
  • If ever needed, PerformanceObserver can be created from native-side

Standards covered:

  • https://www.w3.org/TR/performance-timeline
  • https://www.w3.org/TR/event-timing/
  • https://w3c.github.io/timing-entrytypes-registry
  • https://w3c.github.io/user-timing/

Changelog:

[GENERAL] [CHANGED] - Refactored performance-timeline module, which should improve performance of performance.clearMarks and performance.clearMeasures

Test Plan:

n/a

robik avatar Jun 27 '24 13:06 robik

Pushed early changes for approach review. Still working on updating TurboModule and its spec to match new API.

robik avatar Jul 10 '24 11:07 robik

Ready for an early review. Will undo changes related to exposing Performance TurboModule and still needs some testing from my side.

robik avatar Jul 24 '24 14:07 robik

@rubennorte I've allowed myself to resolve comments that I think are corrected :) The remaining ones I still need some input. Sorry for large batch of changes again, but I think I simplified a lot of things.

FYI: still not fully tested on my side 😅

robik avatar Aug 08 '24 16:08 robik

@rubennorte has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Sep 20 '24 13:09 facebook-github-bot

@rubennorte has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Sep 23 '24 10:09 facebook-github-bot

@rubennorte merged this pull request in facebook/react-native@38b3b2195c2a2839f1751edb8e0d1855b18ac5bf.

facebook-github-bot avatar Sep 26 '24 16:09 facebook-github-bot

This pull request was successfully merged by @robik in 38b3b2195c2a2839f1751edb8e0d1855b18ac5bf

When will my fix make it into a release? | How to file a pick request?

react-native-bot avatar Sep 26 '24 16:09 react-native-bot