[Perf Tracks]: Clear potentially large measures #34803
Conversation
| }; | ||
|
|
||
| const resuableChangedPropsEntry = ['Changed Props', '']; | ||
| const reusableChangedPropsEntry = ['Changed Props', '']; |
eps1lon
left a comment
eps1lon
left a comment
There was a problem hiding this comment.
The only drawback I can think of right now is that they won't be available in performance.getEntries() and PerformanceObserver, but that doesn't really make any sense for DEV envs.
Why not? These APIs can be used to display the data in custom profiler UIs that want more control over what and how they display data.
Is Chrome indefinitely holding onto these entries even if no PerformanceObserver is registered?
It is technically feasible, I am just not sure why would someone do it via User Timings in DEV.
I am not sure, but it looks like so. Having an Observer doesn't imply clearing buffered entries, so I guess we need to do it manually. I think we should do it after every I've validated that clearing measures doesn't affect availability of entries to |
hoxyq
force-pushed
the
perf-tracks/clear-expensive-measures
branch
from
0d9a0b5 to
41bc2de
Compare
So it's just the Though I don't see how the linked spec text confirms that. |
Sorry, summary was not up to date, actually
The value is pushed to the observed entries list as part of the |
hoxyq
force-pushed
the
perf-tracks/clear-expensive-measures
branch
from
41bc2de to
11a58a3
Compare
hoxyq
force-pushed
the
perf-tracks/clear-expensive-measures
branch
from
11a58a3 to
b42f3b2
Compare
Fixes #34770. We need to clear measures at some point, otherwise all these copies of props that we end up recording will allocate too much memory in Chromium. This adds `performance.clearMeasures(...)` calls to such cases in DEV. Validated that entries are still shown on Performance panel timeline. DiffTrain build for [b9ec735](b9ec735)
Fixes #34770. We need to clear measures at some point, otherwise all these copies of props that we end up recording will allocate too much memory in Chromium. This adds `performance.clearMeasures(...)` calls to such cases in DEV. Validated that entries are still shown on Performance panel timeline. DiffTrain build for [b9ec735](b9ec735)
Fixes facebook#34770. We need to clear measures at some point, otherwise all these copies of props that we end up recording will allocate too much memory in Chromium. This adds `performance.clearMeasures(...)` calls to such cases in DEV. Validated that entries are still shown on Performance panel timeline. DiffTrain build for [b9ec735](facebook@b9ec735)
Fixes facebook#34770. We need to clear measures at some point, otherwise all these copies of props that we end up recording will allocate too much memory in Chromium. This adds `performance.clearMeasures(...)` calls to such cases in DEV. Validated that entries are still shown on Performance panel timeline. DiffTrain build for [b9ec735](facebook@b9ec735)
Summary: # Changelog: [Internal] React started using `clearMeasures` in facebook/react#34803. It only does it if `performance.measure()` is defined. This should be enough for a feature check of User Timings API presence, but out stub doesn't follow the same spec. Adding `clearMarks()` and `clearMeasures()` stubs to the object. Differential Revision: D85082720
) Summary: Pull Request resolved: #54214 # Changelog: [Internal] React started using `clearMeasures` in facebook/react#34803. It only does it if `performance.measure()` is defined. This should be enough for a feature check of User Timings API presence, but out stub doesn't follow the same spec. Adding `clearMarks()` and `clearMeasures()` stubs to the object. Reviewed By: rubennorte Differential Revision: D85082720 fbshipit-source-id: 3b117a6545e131cdbb2d5efb73d500a928469864
3 participants
Fixes #34770.
We need to clear measures at some point, otherwise all these copies of props that we end up recording will allocate too much memory in Chromium. This adds
performance.clearMeasures(...)calls to such cases in DEV.Validated that entries are still shown on Performance panel timeline.