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

feat(node): Use worker thread for ANR detection #9789

Closed
wants to merge 9 commits into from

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Dec 9, 2023

I was looking to add more functionality to the ANR debugger but I've been having niggling feelings surrounding our use of a child process to do this. After much experimenting and debugging I managed to get it working via a worker thread and the inspector APIs connectToMainThread().

Also closes #9324

Pros:

  • No extra processes
  • Only ~20MB memory overhead (vs at least 60MB for a child process)
  • Uses inspector API so we can remove the websockets implementation

Cons:

@timfish timfish force-pushed the fix/anr-worker-thread branch from 617dbaa to ed4a316 Compare December 9, 2023 15:58
@timfish
Copy link
Collaborator Author

timfish commented Dec 9, 2023

Now it's working via a worker thread, I'm tempted to resurrect my original load worker from base64 string idea 🤔

This has some pros:

  • Will work in Electron since the worker is just pre-bundled Node.js code
  • ANR can become just be an integration
  • Lower memory usage since the worker doesn't have all the app imports
  • Less chance of running the app twice

And some cons:

  • Minimum supported Node version becomes v14.7
  • Additional bundle size increase of worker code
  • We'll need to pass all the context through to the worker since it will no longer be running the full sdk

@AbhiPrasad AbhiPrasad requested a review from anonrig December 11, 2023 14:25
@timfish
Copy link
Collaborator Author

timfish commented Dec 13, 2023

Closing in favour of #9823

@timfish timfish closed this Dec 13, 2023
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.

Node ANR tracking not getting trace context attached
2 participants