Kudo / react-native-v8

Opt-in V8 runtime for React Native Android
MIT License
914 stars 69 forks source link

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

Closed janicduplessis closed 2 years ago

janicduplessis commented 2 years ago

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 commented 2 years ago

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

shamilovtim commented 2 years ago

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

Szymon20000 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Might be worth a PR into the README