Skip to content

Conversation

@vmoroz
Copy link
Member

@vmoroz vmoroz commented Dec 21, 2022

Description

Enable Hermes sampling profiler for RNW project.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Why

Currently the Hermes sampling profiler does not work.
It prevents any related perf investigations.

What

The change has the following parts:

  • We use new ABI-safe Hermes API based on C. Not all interactions with Hermes are ABI-safe yet. This is just a beginning.
  • The HermesShim is changed to use the new API.
  • We store HermesRuntimeHolder in the context properties. It allows us to add current Hermes runtime for the sampling profiling when it starts and then remove it when it is done.
  • We must add and remove the runtime for sampling profiler in JS dispatcher queue. In this PR we expose the JS dispatcher from the ReactDispatcher.
  • New HermesRuntimeHolder methods loadFrom and storeTo are defined to keep the holder in the context properties.
  • The HermesRuntimeHolder instance is stored on RN instance creation and then removed on RN instance destruction.

Testing

The new sampling profiler is tested manually.
I used the Playground application with Hermes engine.
The menu has commands to start and stop the sampling profiler.
IMPORTANT: the resulting CPU sampling file in the application local storage cannot be opened directly in Chrome or Edge browsers. It must be first post-processed by hermes-profile-transformer tool.

Microsoft Reviewers: Open in CodeFlow

@vmoroz vmoroz requested review from a team as code owners December 21, 2022 22:36
@vmoroz vmoroz force-pushed the PR/FixHermesSampligProfiler_ branch from ba7272a to e966b51 Compare February 3, 2023 15:07
@vmoroz vmoroz force-pushed the PR/FixHermesSampligProfiler_ branch from e966b51 to 228c190 Compare February 8, 2023 17:08
@vmoroz vmoroz requested a review from a team as a code owner February 8, 2023 17:08
@vmoroz vmoroz changed the title [DO NOT MERGE YET] Fix Hermes sampling profiler Fix Hermes sampling profiler Feb 8, 2023
@vmoroz vmoroz added the AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity) label Feb 9, 2023
@vmoroz vmoroz merged commit 08f6ac5 into microsoft:main Feb 9, 2023
@vmoroz vmoroz deleted the PR/FixHermesSampligProfiler_ branch February 9, 2023 18:38
vmoroz added a commit to vmoroz/microsoft-react-native-windows that referenced this pull request Feb 10, 2023
vmoroz added a commit to vmoroz/microsoft-react-native-windows that referenced this pull request Feb 13, 2023
vmoroz added a commit that referenced this pull request Feb 13, 2023
* Fix Hermes sampling profiler (#11033)

* Change files

* Fix Desktop compilation issues

* Fix Hermes version

* Restore telemetry tests

* Fix AccessibilityInfoExample

---------

Co-authored-by: Vladimir Morozov <[email protected]>
Co-authored-by: Andrew Coates <[email protected]>
vmoroz added a commit to vmoroz/microsoft-react-native-windows that referenced this pull request Jun 14, 2023
acoates-ms pushed a commit that referenced this pull request Jun 14, 2023
* Fix Hermes sampling profiler (#11033)

* ABI-safe Hermes API and new JSI for Node-API (#11696)

* Update change files

* Update change file

* Fix compilation issues

* Fix Desktop compilation

* A few minor updates to .filters and comments
vmoroz added a commit to vmoroz/microsoft-react-native-windows that referenced this pull request Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants