PerformanceObserver: default durationThreshold to 104ms#56506
Closed
rubennorte wants to merge 1 commit intofacebook:mainfrom
Closed
PerformanceObserver: default durationThreshold to 104ms#56506rubennorte wants to merge 1 commit intofacebook:mainfrom
rubennorte wants to merge 1 commit intofacebook:mainfrom
Conversation
Summary: React Native's `PerformanceObserver` previously defaulted `durationThreshold` to 0 for `event` entries, while the W3C Event Timing spec (https://www.w3.org/TR/event-timing/#sec-modifications-perf-timeline) mandates a default of 104ms. The previous default flooded observers with short events (clicks, pointer up/down) that are not interesting for responsiveness measurement. This change brings React Native in line with the spec by making 104ms the default. The default is applied at the JSI bridge boundary in `NativePerformance.cpp` (the JS API boundary on the C++ side) when JS does not provide an explicit `durationThreshold`. The C++ public observer API (`PerformanceObserverObserveSingleOptions::durationThreshold`) and the global event buffer (`eventBuffer_.durationThreshold`) keep their `HighResDuration::zero()` default — applying 104ms to the global buffer would drop sub-104ms events at the source, breaking observers that explicitly request `durationThreshold: 0`. Changes in `react-native`: - `NativePerformance.cpp`: apply 104ms default at the JSI bridge boundary. - `PerformanceApiExample.js` (rn-tester): pass `durationThreshold: 0` to preserve previous behavior. - `EventTimingAPI-itest.js`: migrate 4 existing call sites to `{type: 'event', durationThreshold: 0}` and add a new test verifying the 104ms default. Sites using `{entryTypes: ['event']}` were converted to `{type: 'event', durationThreshold: 0}` since the spec (and existing JS validation in `PerformanceObserver.js`) disallows `durationThreshold` together with `entryTypes`. Changelog: [General][Fixed] - PerformanceObserver: `observe({type: 'event'})` now correctly defaults `durationThreshold` to 104ms per the W3C Event Timing spec instead of reporting all events. Differential Revision: D101629586
meta-cla
bot
added
the
CLA Signed
|
@rubennorte has exported this pull request. If you are a Meta employee, you can view the originating Diff in D101629586. |
Collaborator
|
This pull request was successfully merged by @rubennorte in 65c561e When will my fix make it into a release? | How to file a pick request? |
|
This pull request has been merged in 65c561e. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
React Native's
PerformanceObserverpreviously defaulteddurationThresholdto 0 forevententries, while the W3C Event Timing spec(https://www.w3.org/TR/event-timing/#sec-modifications-perf-timeline) mandates a default of 104ms. The previous default flooded observers with short events
(clicks, pointer up/down) that are not interesting for responsiveness
measurement.
This change brings React Native in line with the spec by making 104ms the
default. The default is applied at the JSI bridge boundary in
NativePerformance.cpp(the JS API boundary on the C++ side) when JS does notprovide an explicit
durationThreshold. The C++ public observer API(
PerformanceObserverObserveSingleOptions::durationThreshold) and the globalevent buffer (
eventBuffer_.durationThreshold) keep theirHighResDuration::zero()default — applying 104ms to the global buffer would drop sub-104ms events at
the source, breaking observers that explicitly request
durationThreshold: 0.Changes in
react-native:NativePerformance.cpp: apply 104ms default at the JSI bridge boundary.PerformanceApiExample.js(rn-tester): passdurationThreshold: 0topreserve previous behavior.
EventTimingAPI-itest.js: migrate 4 existing call sites to{type: 'event', durationThreshold: 0}and add a new test verifying the104ms default.
Sites using
{entryTypes: ['event']}were converted to{type: 'event', durationThreshold: 0}since the spec (and existing JSvalidation in
PerformanceObserver.js) disallowsdurationThresholdtogetherwith
entryTypes.Changelog: [General][Fixed] - PerformanceObserver:
observe({type: 'event'})now correctly defaultsdurationThresholdto 104ms per the W3C Event Timing spec instead of reporting all events.Differential Revision: D101629586