Stop creating Owner Stacks if many have been created recently #32529
Conversation
github-actions
bot
added
the
React Core Team
|
|
||
| let recentlyCreatedOwnerStacks = 0; | ||
| let recentlyCreatedDebugTasks = 0; | ||
| setInterval(() => { |
There was a problem hiding this comment.
Can we move the counter and timer to isomorphic react?
It would also be good to reset the counter every commit (and reset the timer too, but maybe not start it until the next render).
There was a problem hiding this comment.
Do we not mean react with "isomorphic react"?
There was a problem hiding this comment.
This interval will be duplicated across jsx and createElement since they're in separate bundles. So it's two.
However, that seems fine. If you have a run away number of elements you'll have that with either createElement or jsx.
Better than leaking this info outside the module and having to read it from an object.
The reason to do isomorphic would be if it was reset using commit or something in each renderer. Although if you have a bunch of frequent commits, maybe it's not so much better.
It does seem weird to have a this running all the time even when React isn't rendering though.
Also, this needs to be DEV-only gated and feature detected if setInterval exists.
There was a problem hiding this comment.
It does seem weird to have a this running all the time even when React isn't rendering though.
That would require adding a dedicated method to internals, right? Should we just share a method to reset with renderers and they decide when to reset (timer-based, commit based etc.) or should it just be a a simple start/stop interval?
eps1lon
force-pushed
the
sebbie/owner-stack-cutoff
branch
from
f134542 to
735d7cf
Compare
eps1lon
force-pushed
the
sebbie/owner-stack-cutoff
branch
from
4551dfd to
6fbd265
Compare
eps1lon
force-pushed
the
sebbie/owner-stack-cutoff
branch
from
6fbd265 to
bc9f075
Compare
eps1lon
force-pushed
the
sebbie/owner-stack-cutoff
branch
from
bc9f075 to
c7fdaef
Compare
Add prestart
Adding Owner Stacks in development adds some non-negligible performance overhead. This is shared roughly equally between the work required for `captureOwnerStack` to work and tasks being visible in runtimes that support `console.createTask`. We now stop this work after a some amount of elements were created. Though we do reset that limit occasionally so that one-off updates on not too large trees do get complete Owner Stacks. Chances are that you probably don't need Owner Stacks on large, frequent updates. Stopping after 10.000 elements caps a large update at 500ms where uncapped would've taken 3.000ms (see attached fixture). I added separate, dynamic feature flags to control when we cut off and how frequent we reset so that Meta can experiment with different values.
Not sure about this one. It'll point into React code i.e. node_modules and should be ignore-listed by modern frameworks.
eps1lon
force-pushed
the
sebbie/owner-stack-cutoff
branch
from
c7fdaef to
dbaf445
Compare
Summary
Adding Owner Stacks in development adds some non-negligible performance overhead.
This is shared roughly equally between the work required for
captureOwnerStackto work and tasks being visible in runtimes that supportconsole.createTask.We now stop this work after a some amount of elements were created. Though we do reset that limit occasionally so that one-off updates on not too large trees do get complete Owner Stacks.
Chances are that you probably don't need Owner Stacks on large, frequent updates.
Stopping after 10.000 elements caps a large update at 500ms where uncapped would've taken 3.000ms (see attached fixture).
I added separate, dynamic feature flags to control when we cut off and how frequent we reset so that Meta can experiment with different values.
How did you test this change?