oneTBB icon indicating copy to clipboard operation
oneTBB copied to clipboard

`tbb::task_arena::enqueue` should at least document that execution order isn't guaranteed

Open BenFrantzDale opened this issue 1 year ago • 5 comments

I was surprised to find that enqueueing into a tbb::task_arena{1} doesn't execute in FIFO order: https://godbolt.org/z/qz1hq3zaE (Although I realize now that for tbb::task_arena{1} I have arena.max_concurrency() == 2, which is not what I'd expect...)

At very least, the docs for tbb::task_arena::enqueue should be clear about this.

At best, it should be that there's a way to make a tbb::task_arena that I can enqueue FIFO work into and have order guaranteed for the case of max_concurrency == 1.

BenFrantzDale avatar Feb 15 '24 16:02 BenFrantzDale

Hi @BenFrantzDale, there is no mention of FIFO guarantee into enqueue specification. arena.max_concurrency() was improved for a case tbb::task_arena{1,0} (in #1128) but tbb::task_arena{1} will still require additional slot for a worker thread because of enqueue logic support thus arena.max_concurrency() == 2.

pavelkumbrasev avatar Feb 16 '24 09:02 pavelkumbrasev

@pavelkumbrasev Thanks. From those docs I'm still not clear what reserved_for_masters means and how max_concurrency and reserved_for_masters interact to produce the int max_concurrency() const result.

I know there's no mention of a FIFO guarantee in enqueue. I'm suggesting that I wished it would affirmatively state that there is no guarantee, especially with a name like "enqueue" versus, say, submit or detach which don't have the same potential FIFO impliciations. In contrast, it does say "Caution: There is no guarantee that tasks enqueued into an arena execute concurrently with respect to any other tasks there." which I read and think: Obviously, there may be only one worker available, and in general, execution order won't be guaranteed because one worker could pick up a task, then immediately go to sleep, then another could pick up the "next" one and do it to completion, giving the effect of the latter one executing first.

I'd be happy to PR documentation changes to clarify, but I'm not sure what the rules actually are.

And in general, if you know a way to set tbb::task_arena or other things up to guarantee FIFO execution (never concurrent), I'm all ears.

BenFrantzDale avatar Feb 16 '24 13:02 BenFrantzDale

Hi @BenFrantzDale, We add extra slot for worker thread in case of tbb::task_arena{1} to support fire-and-forget logic for enqueue tasks. So such arena will have increased concurrency.

You can submit PR into oneTBB specification. There is a link to a PR as an example.

pavelkumbrasev avatar Feb 19 '24 10:02 pavelkumbrasev

@BenFrantzDale Is this is still relevant?

arunparkugan avatar Aug 13 '24 08:08 arunparkugan

It's been a while since I thought about this one, and I'm not finding the HTML docs for tbb::task_arena. I think what I'm asking for is just to add to the existing (doxygen)[https://github.com/wjakob/tbb/blob/master/include/tbb/task_arena.h#L376C1-L379C1] with a note to positively state that there is no guarantee. Like

     //! Enqueues a task into the arena to process a functor, and immediately returns.
     //! Does not require the calling thread to join the arena
+    //! Note: Order of execution is not guaranteed, even if the arena is constructed
+    //!       with max_concurrency == 1.

I opened this ticket because I found it surprising, and I wound up having to do something round-about to get the behavior I wanted: fire-and-forget in-order execution of items on a queue, one at a time.

BenFrantzDale avatar Aug 13 '24 20:08 BenFrantzDale