Make gtl_threadNum invalid by default.

[eugeneko]

I am designing generic API for the users on top of enkiTS. It would be great to tell apart threads managed by enkiTS from other threads. The cheapest (but possibly breaking) way is to make gtl_threadNum = 0xffffffff by default. The compatible way is to add one more thread local flag which is explicitly true for managed threads and false for others. I can make PR if you approve either way.

[dougbinks]

The cleanest would be gtl_threadNum = ENKI_INVALID_THREADNUM with ENKI_INVALID_THREADNUM = 0xffffffff, and then assert in functions which use gtl_threadNum.

I've been thinking about doing this, but a little torn by not wanting to break code. As you mention we could first introduce an extra flag, assert on this in the same way as above, and move to a single gtl_threadNum later.

I'll have a think on this and get back to you shortly.

[eugeneko]

While you are here, two somewhat-related questions.

1. Any reason for GetThreadNum not to be static? It provides access to the static global variable and it will not change in forseeable future.
2. Is // thread_local not well supported yet by C++11 compilers. still relevant? It's been 8 years. It is also technically Undefined Behavior to #define keyworks like thread_local

[dougbinks]

1. This could be static. When I was writing enkiTS I was considering how to handle multiple instances and was considering making the threadnum a member variable. This change could still happen, so unless there's a good use case I'd rather not change it (I am a conservative programmer in general).
2. Generally speaking I try to keep as wide support as possible. Since thread_local wasn't defined on the systems where the define is used it wasn't undefined behaviour, though I note Xcode 8 introduced thread_local in 2016 so I need to add a test for that.

As for gtl_threadNum I note my 'UserThread' branches (developed for a customer wanting to run enkiTS on top of arbitrary schedulers as well as on it's own in testing) use the following:

static const uint32_t                                   NO_THREAD_NUM = 0xFFFFFFFF;
static thread_local uint32_t                            gtl_threadNum = NO_THREAD_NUM;

So I think we should go with this solution. I will merge to the dev branch initially and warn folk of a potential breaking change they should test in debug on this branch.

Since for this use case we'd want NO_THREAD_NUM to be public I think it should either be a static member of the TaskScheduler or we should use ENKI_NO_THREAD_NUM (the later will be needed for the C interface).

[eugeneko]

When I was writing enkiTS I was considering how to handle multiple instances and was considering making the threadnum a member variable.

Just curious, how would that work? You cannot have non-static thread_local member, and re-inventing thread_local manually sounds hard and possibly less efficient.

Since thread_local wasn't defined on the systems where the define is used it wasn't undefined behaviour, though I note Xcode 8 introduced thread_local in 2016 so I need to add a test for that.

Yeah, I use thread_local in Mac/iOS builds in my project just fine, so it is definetely a keyword there. I can always do a local patch in my fork of enkiTS, but I wonder if something can be changed here. Maybe check XCode version if it's possible, or compile time switch going from CMake (like priority count).

[dougbinks]

Just curious, how would that work? You cannot have non-static thread_local member, and re-inventing thread_local manually sounds hard and possibly less efficient.

The idea here was to use the static thread local gtl_threadNum for each instance, with new instances storing a uint32_t mainThreadNum and uint32_t threadNumStart, then computing the internal thread number from this.

I can always do a local patch in my fork of enkiTS, but I wonder if something can be changed here.

Yes, as I mentioned I need to add a test (likely on XCode version as many users don't use CMake, though I'll also add a ENKI_THREAD_LOCAL_DEFINED so this can also be set in a build system). I'll do that soon.

[eugeneko]

The idea here was to use the static thread local gtl_threadNum for each instance, with new instances storing a uint32_t mainThreadNum and uint32_t threadNumStart, then computing the internal thread number from this.

Oh, I didn't think of that, that's nice idea. Thanks. I will be writing my code with the possibility of such change in mind.

[dougbinks]

Marking as complete now this is merged to dev branch. Please reopen if you still have issues.

[eugeneko]

I wanted to say that I really like enkiTS. I tried to find decent task library for a while, but they were all too heavy and with obscure allocation policies (TBB, taskflow, etc). And here is enkiTS -- single-file library that just does the hardest part of the MT tasking, and nothing else. It looks perfect for making existing system efficiently multi-threaded.

[dougbinks]

Thanks! The aim is to try to keep enkiTS as simple as possible, though there remain a few features which are likely useful and don't add too much code.