Skip to content

wall profiler: use thread-local for the active profiler#322

Draft
szegedi wants to merge 1 commit into
mainfrom
szegedi/thread-local-profiler
Draft

wall profiler: use thread-local for the active profiler#322
szegedi wants to merge 1 commit into
mainfrom
szegedi/thread-local-profiler

Conversation

@szegedi
Copy link
Copy Markdown

@szegedi szegedi commented May 11, 2026

Summary

  • Replace the ProtectedProfilerMap (an unordered_map<Isolate*, WallProfiler*> shielded by an atomic-exchange dance so the SIGPROF handler could read it lock-free) with a thread_local WallProfiler* t_active_profiler. In Node's threading model each V8 isolate is pinned to a single thread, so a per-thread pointer captures exactly the fact the signal handler needs without a map, or atomics, or copy-on-write churn on every start/stop. This is the same approach we adopted in the equivalent Node.js core port (Attach values to CPU samples nodejs/node#63163).
  • Keep a separate ActiveProfilers registry (plain std::mutex + std::unordered_set<WallProfiler*>) for the one cross-thread path that survives: the main thread gathering worker-thread CPU while building a profile. That path has no signal-safety requirement, so the concurrency machinery was eliminated from there as well except a simple mutex.
  • Dispose loses its removeFromMap parameter — it now always removes itself from the registry and clears the TLS pointer. CreateV8CpuProfiler checks the TLS to refuse a duplicate start instead of relying on the map's emplace returning false.

This simplifies the SIGPROF handler, the start/stop hot path no longer copies a map under a mutex. This also solves the problem of one thread's SIGPROF handler not being able to find its own map while another is added to it, which would've lost us processing of samples.

Test plan

  • npm run build — clean
  • npm test — 88/88 passing, including Worker Threads ("should work", "should not crash when worker is terminated") which exercises cross-thread CPU gather and destroy-during-profiling
  • npm run format — clean
  • CI green

…er map

The signal handler previously looked up the per-isolate WallProfiler via
ProtectedProfilerMap — an unordered_map shielded by an atomic-exchange dance
so the SIGPROF handler could read it without taking a mutex. In Node's
threading model each V8 isolate is pinned to a single thread, so a
thread_local pointer captures exactly the same fact with no map, no atomics
and no copy-on-write churn on every profiler start/stop.

The cross-thread "gather worker CPU on the main thread" use case still needs
a registry (thread-locals are per-thread), but that path has no
signal-safety requirement, so it collapses to a plain mutex around an
unordered_set plus the terminated-worker CPU bucket.

Dispose no longer needs the removeFromMap parameter — it always removes
itself from the registry and clears the thread-local.
@github-actions
Copy link
Copy Markdown

Overall package size

Self size: 2 MB
Deduped: 2.36 MB
No deduping: 2.36 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | source-map | 0.7.6 | 185.63 kB | 185.63 kB | | pprof-format | 2.2.1 | 163.06 kB | 163.06 kB | | node-gyp-build | 4.8.3 | 13.81 kB | 13.81 kB |

🤖 This report was automatically generated by heaviest-objects-in-the-universe

@szegedi szegedi added the semver-patch Bug or security fixes, mainly label May 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-patch Bug or security fixes, mainly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant