core:thread get_name/set_name

[peperronii]

feature request #5314

tested on arm_darwin

[peperronii]

Actually I’m not sure about this API call. per documentation it might be ^PWSTR but that doesn’t make sense cause it’s a pointer to a pointer.

[blob1807]

It's a pointer to a pointer because it's writing the buffer's pointer at the one we give it. It doesn't copy the whole buffer back to you. It just returns a pointer to it.

[peperronii]

Do we have to still manually allocate a buffer for the entire string or we’ll be fine just using the pointer?
edit:I forgot that we’d have to allocate for the returned string anyway, but I’m still curious how it will manage the buffer after the call.

[blob1807]

It's a pointer to a pointer because it's writing the buffer's pointer at the one we give it. It doesn't copy the whole buffer back to you. It just returns a pointer to it.

[laytan]

With macos not supporting setting the name of another thread, should we just not support it anywhere, so we have a consistent API?

[FourteenBrush]

With macos not supporting setting the name of another thread, should we just not support it anywhere, so we have a consistent API?

I'm wondering whether adding a possibility to thread.create_*/thread.start to add a name for the newly created thread would be an idea, instead of adding a thread.set_name proc?

Similar to how Java does it with new Thread(Runnable, name), this way thread creation/starting could set the name upfront, as this name is merely a "debugging" thing and I don't see a reason why someone would arbitrarily change one.

[laytan]

With macos not supporting setting the name of another thread, should we just not support it anywhere, so we have a consistent API?

I'm wondering whether adding a possibility to thread.create_*/thread.start to add a name for the newly created thread would be an idea, instead of adding a thread.set_name proc?

Similar to how Java does it with new Thread(Runnable, name), this way thread creation/starting could set the name upfront, as this name is merely a "debugging" thing and I don't see a reason why someone would arbitrarily change one.

Good idea!

[peperronii]

With macos not supporting setting the name of another thread, should we just not support it anywhere, so we have a consistent API?

Yeah, I was also hesitant about this but It feels weird to shun others capability just to make this one feels consistent. I think the solution to have all the OS refer to the calling thread is pretty neat though. but, I would gladly comply if it’s somehow idiomatic.

I'm wondering whether adding a possibility to thread.create_*/thread.start to add a name for the newly created thread would be an idea, instead of adding a thread.set_name proc?

Similar to how Java does it with new Thread(Runnable, name), this way thread creation/starting could set the name upfront, as this name is merely a "debugging" thing and I don't see a reason why someone would arbitrarily change one.

I also think this is a good idea. but, I’m not sure about not adding thread.set_name at all though. even though I don’t personally use it.

@GloriousPtr thoughts?

[FourteenBrush]

I would still get rid of the set_name, and instead move this name to thread.create, where something like __unix_thread_entry_proc and similar would set the name on the running thread itself

[FourteenBrush]

@laytan I just looked at how Zig does it, and it simply returns an error.Unsupported, perhaps we should just also do that? 🙂

https://ziglang.org/documentation/master/std/#std.Thread.setName

Seems like a reasonable tradeoff right?, additionally still keep the idea of an initial thread name param.

[peperronii]

If we’re going down the thread.create_with_name* route, I think we don’t need to have a get_name. we have to make a struct field (thread.Thread) for the name to make it callable from inside the thread, so if we want to retrieve the name, we can just t.name or something like that.

[FourteenBrush]

I think we don’t need to have a get_name. we have to make a struct field (thread.Thread) for the name to make it callable from inside the thread

I disagree, I don't see a reason to add this field to a Thread, the kernel already represents the single point of truth for this value.
This isn't a hot spot either so in my opinion nothing wrong with performing a syscall here to obtain the value.

My idea was to do something like this:

thread.create :: proc(procedure: Thread_Proc, priority: Thread_Priority = Thread_Priority.Normal, name: Maybe(string) = nil) -> ^Thread

(Similarly for all those other variants)
Which then calls the respective _set_name proc in the threads __entry_proc, as this would get rid of the issue that darwin does not support changing arbitrary thread's names

[peperronii]

I disagree, I don't see a reason to add this field to a Thread, the kernel already represents the single point of truth for this value. This isn't a hot spot either so in my opinion nothing wrong with performing a syscall here to obtain the value.

yeah, I suppose you’re right on not getting rid of get_thread. But we do need to add a field in thread.Thread in order to pass it inside __unix_thread_entry_proc and __windows_thread_entry_proc though… right?

[FourteenBrush]

But we do need to add a field in thread.Thread in order to pass it inside __unix_thread_entry_proc and __windows_thread_entry_proc though… right?

Looks like it, can't use context._internal here because the windows thread entry proc may not be called immediately

[flysand7]

Please add the note for all procedures that take in a name for creating a thread/setting the thread name that name parameter is truncated to 16 characters on unix and 64 characters on windows.

[flysand7]

Instead of creating 5 files for each of the platforms, I think you should just add these bindings to sys/posix/pthread.odin. They don't seem to be platform specific and there's no point in duplicating the files when you can bind these procedures in one place.

[peperronii]

1. it is actually platform specific, the input arguments for the foreign functions are not the same. I actually wanted to put it in the same file but I’m not sure if we can do something like a WHEN ODIN_OS == .smth inside a foreign block though, so I went with this.

2. These functions are not listed in the POSIX standard. I’ve been informed that the core:sys/posix adheres to it very strictly.

[flysand7]

Okay, then it's good. I'm not familiar with posix much, I assumed it would have been part of pthreads

[FourteenBrush]

Looks fine to me, is it tested on all platforms?

[peperronii]

reviewed and tested on macos_arm using thread.create and thread.get_name.