Implement Runtime.getHeapUsage for hermes chrome inspector#33173
Closed
janicduplessis wants to merge 3 commits intofacebook:mainfrom
Closed
Implement Runtime.getHeapUsage for hermes chrome inspector#33173janicduplessis wants to merge 3 commits intofacebook:mainfrom
janicduplessis wants to merge 3 commits intofacebook:mainfrom
Conversation
…32895) Summary: I was looking at the hermes chrome devtools integration and noticed requests to `Runtime.getHeapUsage` which was not implemented. When implemented it will show a summary of memory usage of the javascript instance in devtools. <img width="325" alt="image" src="https://user-images.githubusercontent.com/2677334/149637113-e1d95d26-9e26-46c2-9be6-47d22284f15f.png"> ## Changelog [General] [Added] - Implement Runtime.getHeapUsage for hermes chrome inspector Pull Request resolved: facebook#32895 Test Plan: Before <img width="912" alt="image" src="https://user-images.githubusercontent.com/2677334/149637073-15f4e1fa-8183-42dc-8673-d4371731415c.png"> After <img width="1076" alt="image" src="https://user-images.githubusercontent.com/2677334/149637085-579dee8f-5efb-4658-b0a8-2400bd119924.png"> Reviewed By: christophpurrer Differential Revision: D33616658 Pulled By: ShikaSD fbshipit-source-id: 49d863e6a58d4a92d4c86f9a288ac33ed8d2cb0d
facebook-github-bot
added
the
CLA Signed
facebook-github-bot
added
Contributor
react-native-bot
added
the
Type: Enhancement
Base commit: 189c2c8 |
Base commit: 189c2c8 |
Contributor
|
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Contributor
|
hey, I see this test failing in internal CI: seems like re-serialized version contains param object, could you update the test or change serialization strategy? 🙂 |
janicduplessis
force-pushed
the
profile-heap-info-v2
branch
from
4860924 to
39d2638
Compare
Contributor
Author
|
@ShikaSD I updated the test, I can't run it locally tho since the BUCK config depends on FB internal stuff. |
Contributor
|
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Collaborator
|
This pull request was successfully merged by @janicduplessis in cff9590. When will my fix make it into a release? | Upcoming Releases |
Saadnajmi
pushed a commit
to Saadnajmi/react-native-macos
that referenced
this pull request
Jan 15, 2023
…33173) Summary: Reland of facebook#32895 with fix for optional params object. Original description: I was looking at the hermes chrome devtools integration and noticed requests to `Runtime.getHeapUsage` which was not implemented. When implemented it will show a summary of memory usage of the javascript instance in devtools. <img width="325" alt="image" src="https://user-images.githubusercontent.com/2677334/149637113-e1d95d26-9e26-46c2-9be6-47d22284f15f.png"> [General] [Added] - Implement Runtime.getHeapUsage for hermes chrome inspector Pull Request resolved: facebook#33173 Test Plan: I was able to reproduce the issue that caused the initial PR to be reverted using the resume request in Flipper. Pausing JS execution then resuming it will cause it to crash before this change, and works fine after. Before <img width="912" alt="image" src="https://user-images.githubusercontent.com/2677334/149637073-15f4e1fa-8183-42dc-8673-d4371731415c.png"> After <img width="1076" alt="image" src="https://user-images.githubusercontent.com/2677334/149637085-579dee8f-5efb-4658-b0a8-2400bd119924.png"> Reviewed By: jpporto Differential Revision: D34446672 Pulled By: ShikaSD fbshipit-source-id: 6e26b8d53cd88cddded36437c72a01822551b9d0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reland of #32895 with fix for optional params object.
Original description:
I was looking at the hermes chrome devtools integration and noticed requests to
Runtime.getHeapUsagewhich was not implemented. When implemented it will show a summary of memory usage of the javascript instance in devtools.Changelog
[General] [Added] - Implement Runtime.getHeapUsage for hermes chrome inspector
Test Plan
I was able to reproduce the issue that caused the initial PR to be reverted using the resume request in Flipper. Pausing JS execution then resuming it will cause it to crash before this change, and works fine after.
Before
After