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

Map native thread ids with Ruby thread names? #9

Closed
byroot opened this issue Mar 14, 2023 · 4 comments
Closed

Map native thread ids with Ruby thread names? #9

byroot opened this issue Mar 14, 2023 · 4 comments

Comments

@byroot
Copy link
Contributor

byroot commented Mar 14, 2023

Ruby 3.2 added Thread#native_thread_id

When available I think we could use that to find the Ruby thread and provide more information about the thread.

If Thread#name returns something we could use that, otherwise we can probably report the location of its first frame like Thread#inspect.

cc @benoittgt

@ivoanjo
Copy link
Owner

ivoanjo commented Mar 14, 2023

I like that! I took a very simple stab at it in 944d4d3 but definitely think it'd be interesting to add more.

I guess the main constraint here is being careful that we only read Thread#name or Thread#inspect when we're holding the GVL (or its modern equivalent) as from what I could tell, not all the callbacks get triggered when that happens (...right?).

But definitely suggestions are welcome!

@benoittgt
Copy link
Contributor

Good idea

Also with the native extension (https://github.com/ivoanjo/gvl-tracing/blob/master/ext/gvl_tracing_native_extension/gvl_tracing.c#L90). How much can we reuse from Thread#to_s code?

@byroot
Copy link
Contributor Author

byroot commented Mar 14, 2023

I guess the main constraint here is being careful that we only read Thread#name or Thread#inspect when we're holding the GVL

Absolutely. Ideally we'd just dump the native thread id in the file, and only later do the mapping / rewrite. Some epehmeral threads may have been GCed, but I guess that's acceptable (even more so because ephemeral threads are recycled, so they won't show up properly anyway).

benoittgt added a commit to benoittgt/gvl-tracing that referenced this issue Apr 5, 2023
gvl-tracing handles properly thread name for basic usage like:
```ruby
  Thread.new do
    Thread.current.name = "thread name
  end
```

But on more complicated with external libraries it is not able to
properly handles thread name.
Jean had a good idea in ivoanjo#9
about using `Thread.list`.

This commit does this:
- Store as list of json line, events we want to monitor
- Store `Thread.list` before stopping saving event related to the GVL
- Iterate of events store in json file and modify entries where we can
  find the name of the thread
- Go a little bit further when we can provide the gem name

This is a very naive approach and probably not super efficient. But this
is a first try.
benoittgt added a commit to benoittgt/gvl-tracing that referenced this issue Apr 26, 2023
gvl-tracing handles properly thread name for basic usage like:
```ruby
  Thread.new do
    Thread.current.name = "thread name
  end
```

But on more complicated with external libraries it is not able to
properly handles thread name.
Jean had a good idea in ivoanjo#9
about using `Thread.list`.

This commit does this:
- Store as list of json line, events we want to monitor
- Store `Thread.list` before stopping saving event related to the GVL
- Iterate of events store in json file and modify entries where we can
  find the name of the thread
- Go a little bit further when we can provide the gem name

This is a very naive approach and probably not super efficient. But this
is a first try.
ivoanjo added a commit that referenced this issue Jun 6, 2023
gvl-tracing handles properly handles thread name for basic usage like:
```ruby
  Thread.new do
    Thread.current.name = "thread name"
  end
```

But on more complicated examples with external libraries, it is not able
to properly handles thread name.
Jean had a good idea in #9
with using `Thread.list`.

This commit does this:
- Store as list of json line, events we want to monitor
- Store `Thread.list` before stopping gvl tracing
- Iterate of events stored in json file and modify entries where we can
find the id of a thread
- Go a little bit further when we can and provide the gem name related
to the thread

This is a very naive approach and probably not super efficient. But this
is a first try.

Screenshot with the new examples file:
<img width="1049" alt="Screenshot 2023-04-05 at 21 23 45"
src="https://user-images.githubusercontent.com/8417720/230187851-9816dd5b-9b60-4200-b214-bb6b82c82c31.png">

Screenshot of a `gvl-tracing` on a [Rails app with some
multihread](https://github.com/benoittgt/load_async_rails/blob/main/app/controllers/posts_controller.rb#L6-L11):

<img width="570" alt="Screenshot 2023-04-05 at 21 45 01"
src="https://user-images.githubusercontent.com/8417720/230189055-9ee361b4-f933-4329-878b-3724372ab7a4.png">


Opening as a draft because code need probably some rework.
@ivoanjo
Copy link
Owner

ivoanjo commented Jun 6, 2023

Fixed in #12

@ivoanjo ivoanjo closed this as completed Jun 6, 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

No branches or pull requests

3 participants