[DevTools] RFC: log profiling events under __PROFILE__ flag

[jstejada]

Summary

This commit is being used as proposal for adding a logger (EventLogger) that records events happening in the DevTools runtime. In particular, this code was used for collecting profiling data to measure the performance of parseHookNames.

The current EventLogger can be extended to log and collect different types of events that we can statically enforce with Flow (e.g. for performance metrics, or for other relevant product measurements), and which can then be accessed by various means. Right now, they are just stored in a global constant, but in the future they could be logged to other logging systems (e.g. external logging systems).

Note that when __PROFILE__ flag is disabled, all of the event logging will be disabled and stripped from the build.

Test Plan

• yarn flow
• yarn test
• yarn test-build-devtools
• used this code to collect performance data for parseNamedHooks.

[bvaughn]

Don't have time to review this full PR now, but a quick thought: The __PROFILE__ flag has a specific meaning for React builds already. (There's even a global __PROFILE__ flag defined for DevTools too, in the Webpack configs.)

I think this functionality is a bit different. Maybe we should use a different name to avoid confusion?

[jstejada]

oops, a few unrelated changes got added to this PR (some refactoring of the calls to originalPositionFor) it's still good to take a look but i'll split those up

[bvaughn]

I'm wondering why we are using a global here?

Seems like we could use our feature flag system to create a proper logging (enabled for Facebook builds only) that could batch and flush these to some internal logging system.

[jstejada]

yeah the global was just for manual testing purposes. we don't have to land this global, but i would imagine as a follow up we would do this:

Seems like we could use our feature flag system to create a proper logging (enabled for Facebook builds only) that could batch and flush these to some internal logging system.

[bvaughn]

Fair enough! Works fine for a POC 😁

I think I'd probably want to do this design work as part of the initial PR rather than a follow up though.

[bvaughn]

This function and the withCallbackProfiling function below could be part of such a logger. (Maybe non-Facebook builds would just get a no-op logger that does nothing.)

[supinda2499]

I forwarded the mail to the wrong group. I'm sorry from the bottom of my heart เมื่อ พ. 25 ส.ค. 2564 เวลา 05:58 Brian Vaughn ***@***.***> เขียนว่า:

***@***.**** commented on this pull request. ------------------------------ In packages/react-devtools-extensions/src/parseHookNames/parseHookNames.js <#22167 (comment)>: > @@ -104,12 +128,73 @@ const originalURLToMetadataCache: LRUCache< }, }); +function withSyncProfiling<TReturn>( This function and the withCallbackProfiling function below could be part of such a logger. (Maybe non-Facebook builds would just get a no-op logger that does nothing.) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#22167 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJGPVI7K35WEMAAD6S57Z53T6QP2XANCNFSM5CXLALTA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email> .

-- I-Phone.Su

[jstejada]

abandoning in favor of #22276