Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix StringView value when running devtool message on JS thread #122

Merged
merged 1 commit into from
Jun 24, 2022

Conversation

janicduplessis
Copy link
Contributor

After testing devtools profiler in a large app, we noticed that the profiler stop command would crash with an invalid JSON message error. Upon inspection, what happens is the value of messageView when inside the runOnQueue closure is no longer the right value, which then fails to parse as JSON in the v8 code.

I think StringView is associated to a v8 scope, which would cause it to be garbage collected at the end of the function. Passing the std::string to the closure instead fixes the problem.

@Szymon20000
Copy link
Contributor

Szymon20000 commented Jun 23, 2022

Nice catch!
Hopefully it will just work now :D
It took us few weeks to find and fix all issues related to v8 profiler

Copy link
Owner

@Kudo Kudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice fix thanks @janicduplessis

@Kudo Kudo merged commit f2e6ae6 into Kudo:main Jun 24, 2022
@janicduplessis janicduplessis deleted the patch-2 branch June 24, 2022 12:31
@shamilovtim
Copy link

Are there any docs out there on how to use a profiler with v8?

@Szymon20000
Copy link
Contributor

One of the ways is to just use flipper :)
flipper -> Hermes Profiler -> more tools (3 dots) -> Javascript Profiler (not performance tab) and you are good to go.
I think I will create a twitter thread about it.

@janicduplessis
Copy link
Contributor Author

If you are using latest version of RN it should also be easier since it opens the node version of devtools instead of the browser one so the profiler it shows is the right one

@janicduplessis
Copy link
Contributor Author

I also wonder if we can make it so the react dev menu opens flipper like when using hermes instead of remote devtools. I saw there’s a isInspectable value from the runtime.

@shamilovtim
Copy link

Might be worth a PR into the README

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants