Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ben-clayton opened this issue Mar 16, 2020 · 5 comments
Open

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

ben-clayton opened this issue Mar 16, 2020 · 5 comments

Comments

@ben-clayton
Copy link
Contributor

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

@whalbawi
Copy link
Contributor

Hi @ben-clayton. Wondering what the expected behavior for marl::Ticket::onCall is. The documentation in 0182eb9 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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants