Skip to content

Commit

Permalink
Keep track names
Browse files Browse the repository at this point in the history
Summary:
`threadNames_` is a map from thread id to thread name.

This is the only place where this map is populated:
https://www.internalfb.com/code/fbsource/[9c9ffce4179fbc0c3f05c370646ec159cc4fb6a1]/xplat/hermes/lib/VM/Profiler/SamplingProfiler.cpp?lines=129-132

Since this only happens in a constructor, I don't really understand why it has to be a map. A single thread will be recorded, basically the one on which the instance of SamplingProfiler is created.

We have a `clean()` function, which basically resets the state of `SamplingProfiler` and wipes out everything we could've just emitted via one of the JS Samplings APIs.

This API also cleans thread names map, but this single thread inside this map might still be alive and this SamplingProfiler instance is not destroyed, so this thread entry will never be registered again.

This might be not the best fix for this issue, and I am open to changing the approach:
- Do we really need to keep a map of thread names here?
- Can some methods on `SamplingProfiler` be executed on a thread different from the one where `SamplingProfiler` was created?

My current understanding is that `Sampler` is a global instance, which keeps references to potentially multiple local `SamplingProfiler` instances, which may be on different threads, but `SamplingProfiler` <-> `Thread` relationship is 1 <-> 1.

Reviewed By: mattbfb

Differential Revision: D67453370

fbshipit-source-id: 55dc308c46b0dfa9116904fa2d32c5341940ecd6
  • Loading branch information
hoxyq authored and facebook-github-bot committed Feb 4, 2025
1 parent 37437be commit e90cda2
Showing 1 changed file with 0 additions and 2 deletions.
2 changes: 0 additions & 2 deletions lib/VM/Profiler/SamplingProfiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,6 @@ void SamplingProfiler::clear() {
// Release all strong roots.
domains_.clear();
nativeFunctions_.clear();
// TODO: keep thread names that are still in use.
threadNames_.clear();
}

void SamplingProfiler::suspend(
Expand Down

0 comments on commit e90cda2

Please sign in to comment.