Comparing changes

#31153) Fixes #28984.

…e from backend (#31117) We can't wait for a response from Backend, because it might take some time to actually finish profiling. We should keep a flag on the frontend side, so user can quickly see the feedback in the UI.

#31118) Stacked on #31117. No need for sending long float numbers and to have resolution less than a microsecond, we end up formatting it on a Frontend side: https://github.com/facebook/react/blob/6c7b41da3de12be2d95c60181b3fe896f824f13a/packages/react-devtools-shared/src/devtools/views/Profiler/utils.js#L359-L360

Stacked on #31118. See last commit. We don't need to call `startProfiling()` here, because we delegate this to the Renderer itself: https://github.com/facebook/react/blob/830e823cd2c6ee675636d31320b10350e8ade9ae/packages/react-devtools-shared/src/backend/fiber/renderer.js#L5227-L5232 Since this is de-facto the constructor of Renderer, this will be called earlier. Validated via testing the reload-to-profile for Chrome browser extension.

@EdmondChuiHW

) Based on #31049, credits to @EdmondChuiHW. What is happening here: 1. Once Agent is destroyed, unsubscribe own listeners and bridge listeners. 2. [Browser extension only] Once Agent is destroyed, unsubscribe listeners from BackendManager. 3. [Browser extension only] I've discovered that `backendManager.js` content script can get injected multiple times by the browser. When Frontend is initializing, it will create Store first, and then execute a content script for bootstraping backend manager. If Frontend was destroyed somewhere between these 2 steps, Backend won't be notified, because it is not initialized yet, so it will not unsubscribe listeners correctly. We might end up duplicating listeners, and the next time Frontend is launched, it will report an issues "Cannot add / remove node ...", because same operations are emitted twice. To reproduce 3 you can do the following: 1. Click reload-to-profile 2. Right after when both app and Chrome DevTools panel are reloaded, close Chrome DevTools. 3. Open Chrome DevTools again, open Profiler panel and observe "Cannot add / remove node ..." error in the UI.

Stacked on #31131. See last commit. This is a clean-up and a pre-requisite for next changes: 1. `ReloadAndProfileConfig` is now split into boolean value and settings object. This is mainly because I will add one more setting soon, and also because settings might be persisted for a longer time than the flag which signals if the Backend was reloaded for profiling. Ideally, this settings should probably be moved to the global Hook object, same as we did for console patching. 2. Host is now responsible for reseting the cached values, Backend will execute provided `onReloadAndProfileFlagsReset` callback.

Stacked on #31132. See last commit. There are 2 issues: 1. We've been recording timeline events, even if Timeline Profiler was not supported by the Host. We've been doing this for React Native, for example, which would significantly regress perf of recording a profiling session, but we were not even using this data. 2. Currently, we are generating component stack for every state update event. This is extremely expensive, and we should not be doing this. We can't currently fix the second one, because we would still need to generate all these stacks, and this would still take quite a lot of time. As of right now, we can't generate a component stack lazily without relying on the fact that reference to the Fiber is not stale. With `enableOwnerStacks` we could populate component stacks in some collection, which would be cached at the Backend, and then returned only once Frontend asks for it. This approach also eliminates the need for keeping a reference to a Fiber.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comparing changes

Open a pull request

Commits on Oct 9, 2024

This comparison is taking too long to generate.