Skip to content

Starlingmonkey timers #798

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

Merged
merged 8 commits into from
Jun 6, 2024
Merged

Starlingmonkey timers #798

merged 8 commits into from
Jun 6, 2024

Conversation

guybedford
Copy link
Contributor

@guybedford guybedford commented Jun 5, 2024

This integrates Fastly support for timers on top of the new deadline() hook on AsyncTask for StarlingMonkey, which is supported by StarlingMonkey timers.

Timers are represented by a handle that is never used in Fastly's host call model. When encountering a timer in the select host hook we check its deadline and progress it if it is ready. We also bound the select call timeout now by the least deadline when timer tasks are present, as we previously implemented.

This architecture also exposes an implementation gap in that setting timers when no other async tasks were pending would never resolve. Without any other primitives to work with a busy loop is implemented in this case to properly support real timeouts and a new test is added. Alternatively, we could throw an error here but I strongly believe we should focus on supporting the code our users write instead of throwing for standard JS patterns.

@guybedford guybedford requested a review from elliottt June 5, 2024 21:37
Copy link
Contributor

@elliottt elliottt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this all looks reasonable! I had some suggestions for small tweaks.

@guybedford guybedford enabled auto-merge (squash) June 6, 2024 00:23
@guybedford guybedford merged commit 57f4423 into main Jun 6, 2024
20 checks passed
@guybedford guybedford deleted the starlingmonkey-timers branch June 6, 2024 00:34
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

Successfully merging this pull request may close these issues.

2 participants