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 the ability to have async steps #83

Open
bgrins opened this issue Mar 3, 2023 · 2 comments
Open

Add the ability to have async steps #83

bgrins opened this issue Mar 3, 2023 · 2 comments
Labels

Comments

@bgrins
Copy link
Contributor

bgrins commented Mar 3, 2023

There are some workloads that we hard to capture synchronously in a BenchmarkTestStep. Areas this has come up

  • Moanco (Two new editor workloads #81 (comment)) since it relies on a worker to do some processing
  • React-Stockcharts (Add react-stockcharts benchmarks #11 (comment)), and @camillobruni has raised some concerns with bimodal distribution on Chrome that is quite possibly related.
  • In the research on data grids I see some grid libraries that delay rendering and so spend 0ms during the step. I've also experimented with and think we should consider using in-memory SQLite WASM as a backend for these tests, and that runs in a worker so async is a must-have.
  • In the research on charting libraries that @julienw has been doing - where certain libraries delay rendering (especially React-based wrappers AIUI) in a way that causes significant work to happen outside of the timed window.

@rniwa and I discussed a bit on WebKit Slack, and concluded

yeah, making the sync step compatible with promise seems okay to me. We just need to make sure we don't accidentally start measuring truly async stuff like network loads and such.

I don't know mechanically what will be involved with a change here, and the main runner function comments about not using Promise

// This function ought be as simple as possible. Don't even use Promise.
. There's also #28 (comment) which may or may not make sense to tackle at the same time if we're going to be tweaking the core measurement function.

@bgrins
Copy link
Contributor Author

bgrins commented Mar 15, 2023

Status update: @camillobruni mentioned last meeting that was on his radar in the coming weeks

@rniwa rniwa added the v4 label May 19, 2023
@rniwa
Copy link
Member

rniwa commented May 19, 2023

Sounds like we want to push this to v4 based on this week's sync?

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

No branches or pull requests

2 participants