google / marl

A hybrid thread / fiber task scheduler written in C++ 11
Apache License 2.0
1.84k stars 191 forks source link

Add test coverage for marl::Ticket::onCall #100

Open ben-clayton opened 4 years ago

ben-clayton commented 4 years ago

This is not currently being tested, and as it is a template method, we don't even check it builds correctly.

whalbawi commented 3 years ago

Hi @ben-clayton. Wondering what the expected behavior for marl::Ticket::onCall is. The documentation in 0182eb968366364288f5b26a77b87f05a9ce4782 says:

// onCall() registers the function f to be invoked when this ticket is
// called. If the ticket is already called prior to calling onCall(), then
// f() will be executed immediately.
// F must be a function of the OnCall signature.
template <typename F>
MARL_NO_EXPORT inline void onCall(F&& f) const;

It seems that the code schedules the callback for deferred execution regardless of state the ticket (called/waiting) at the time of registration. Please let me know if I'm missing something.

ben-clayton commented 3 years ago

Hi @waelhalbawi,

The documentation isn't quite right, in that will be executed immediately should be will be scheduled for execution immediately.

It seems that the code schedules the callback for deferred execution regardless of state the ticket (called/waiting) at the time of registration

This statement doesn't seem right though. If the ticket hasn't been called yet (we get to here), then the function is copied to the record->onCall field for invoking when the ticket is called.

Cheers, Ben

whalbawi commented 3 years ago

Hi @ben-clayton. Thanks for clarifying.

This statement doesn't seem right though. If the ticket hasn't been called yet (we get to here), then the function is copied to the record->onCall field for invoking when the ticket is called.

What I meant to say is that a registered callback is scheduled for execution here, and not invoked in-line, once the ticket is called, which is consistent with your clarification.

Do you have examples of where/how this feature is used?

ben-clayton commented 3 years ago

It's used in SwiftShader, but the code is pretty complex

The basic idea is that you want to avoid starting a fiber that immediately blocks on another sync primitive, as you'll just be allocating a fiber and wasting a bunch of cycles to insta-yield. If you need to wait for a chain of previous events to complete, onCall is handy for deferring some work without an immediate wait.

I've got quite a heavy week on, but I'll see if I can write some tests which can close this bug and act as a basic example.

whalbawi commented 3 years ago

Thanks!