Support ruby 3.2 + 3.3+

[jpcamara]

This is still WIP, but I wanted to open it up so the general approach is clear. Main oddities for me right now:

• Having to free the data at the end of tracing. I guess that's ok? It was the only way I could figure out how to get the new rb_internal_thread_specific_* api working properly without a memory leak
• Name is not going to work correctly in ruby 3.3+ as this code is right now, since the events don't guarantee to fire from their associated thread. Ideally we'd get the name from the thread, but that is kind of tricky since ruby does not expose a method for it, and Thread.list doesn't contain any threads that have exited at the end of the tracing
• I know object_id is considered inefficient (https://github.com/ruby/ruby/pull/9276), but the overhead here seems minimal enough? I like it better as an identifier than a serial id and it lets MN threads work the same as native threads

Commit notes

• There are lots of changes to how Ruby 3.3+ works with thread instrumentation.

• There is no guarantee which thread will trigger hooks. For instance, in Ruby 3.3 the RUBY_INTERNAL_THREAD_EVENT_STARTED event is always called from a different thread

• As a result, you have to use the event_data->thread member in conjunction with rb_internal_thread_specific_get/set to emulate "thread local" data tracking. You can't use actual thread locals anymore because the hooks are not fired on the same thread

• As a result, we can't reliably retrieve the native thread id because ruby does not expose a way to do this and using system calls won't work on different threads. Now uses object_id in ruby 3.3+

• These changes also work better with the M:N thread scheduler, since even if we could access native thread ids the M:N scheduler mostly reuses threads so the associated thread ids would be wrong

• Because the events can fire from different threads, the current way of getting the thread name (pthread_getname_np), will probably be wrong

• Using rb_thread_current() in the instrumentation hook calls causes a segfault, but it seems to work fine in the GC methods and when starting/stopping tracing

• Because we malloc the thread data in ruby 3.3, we free it up when we stop tracing, or whenever a thread exits

[jpcamara]

Just noticed a mistake I'll fix around the thread access in Ruby 3.3, will push that up later today

Windows is still broken which I thought was turned off since it would never work but I'll check it another time

[jpcamara]

Any chance you could weigh in on the approach @byroot?

In terms of the need to manually free, I've started trying to use TypedData_Make_Struct, but don't have it fully working yet and I'm not sure it's the right approach since that seems more meant for externally exposed structs?

Also not sure the best approach for getting thread names to display. Most of my testing involves the threads exiting before the block can pull them from Thread.list so the names are always either wrong or empty

[jpcamara]

I haven't reviewed too deeply, but the changes overall make sense to me.

For a sec I was worried the state allocation in the callback may be racy, but actually it's fine because a given thread can't process two events concurrently, so seems OK to me.

@casperisfine Thanks for the feedback! I'll take a look at GVL tracing and see if I can clean this up a bit based on that.

Do you have any thoughts on a way to track thread names that die before we can call Thread.list at the end?

[byroot]

I'm curious -- is that the case even when not using M:N scheduling on the main ractor? What thread does it get called from? 🤔

Yes, it's fired from the parent thread. That simplified the implementation quite a bit.

[jpcamara]

Hey!

First of all, big thanks for working on this! And for the patience with my sloooow review haha >_>

Having to free the data at the end of tracing. I guess that's ok? It was the only way I could figure out how to get the new rb_internal_threadspecific* api working properly without a memory leak

This seems very fine! In fact, it's better than the previous design, where the data would "live forever" as part of the native thread's thread-local stuff.

Name is not going to work correctly in ruby 3.3+ as this code is right now, since the events don't guarantee to fire from their associated thread. Ideally we'd get the name from the thread, but that is kind of tricky since ruby does not expose a method for it, and Thread.list doesn't contain any threads that have exited at the end of the tracing

Yeah... This is a bit annoying. Maybe something similar to @casperisfine suggested would work: we could keep references to all the threads we've seen (even those that die) and then in stop, we'd use that instead of Thread.list to iterate and get names from.

(E.g. keep it in a C array, that we need to mark but otherwise can operate on without the GVL)

I know object_id is considered inefficient (Reduce usage of object_id in core gems ruby/ruby#9276), but the overhead here seems minimal enough? I like it better as an identifier than a serial id and it lets MN threads work the same as native threads

I think if you create enough threads for object_id calls to add any measurable overhead, you're probably already holding it wrong :rofl: .

There is no guarantee which thread will trigger hooks. For instance, in Ruby 3.3 the RUBY_INTERNAL_THREAD_EVENT_STARTED event is always called from a different thread

I'm curious -- is that the case even when not using M:N scheduling on the main ractor? What thread does it get called from? :thinking:

Thanks for the feedback! I'll make some changes in the next few days so we can get this buttoned up 🎉

[jpcamara]

Closing in favor of https://github.com/ivoanjo/gvl-tracing/pull/18