perf: migrate all blocking call to async call #9217
Conversation
✅ Deploy Preview for rspack ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
github-actions
bot
added
the
release: performance
|
📝 Benchmark detail: Open
Threshold exceeded: ["10000_development-mode + exec","10000_development-mode_hmr + exec","10000_production-mode + exec","10000_production-mode_persistent-cold + exec","10000_production-mode_persistent-hot + exec","arco-pro_development-mode + exec","arco-pro_production-mode + exec","arco-pro_production-mode_persistent-hot + exec","large-dyn-imports_development-mode + exec","large-dyn-imports_production-mode + exec","threejs_development-mode_10x + exec","threejs_development-mode_10x_hmr + exec","threejs_production-mode_10x + exec","threejs_production-mode_10x_persistent-cold + exec","threejs_production-mode_10x_persistent-hot + exec"] |
|
benchmark triggered 😄 |
|
🤣Strange performance regression since my local tests are positive. I will continue to investigate. |
|
@CPunisher notice that async call have performance gain when have lots of async js call but may not have performance gain(may even bring performance regression) if not have any or little js call |
| }) | ||
| .collect::<Vec<_>>(); | ||
| let compilation = &*self; | ||
| let results = join_all(jobs.into_iter().map(|job| async move { |
There was a problem hiding this comment.
this seems run in single thread while rayon runs in multi thread, so you may need to use spawn to run task in multi thread, this may cause performance regression now
There was a problem hiding this comment.
yes, i realize this is not a good migration pattern
There was a problem hiding this comment.
actually I don't know what's the perfect way to migrate these, async parallel is kind of hard in rust now
Summary
Related: #8312
Help me start benchmarking. I will try to polish the code further if performance improves (and split the pr).
There are still undo tasks:
block_ons andblocking_calls are called inimpl From<RawXX> for XXs, which is hard to be converted intoasync. Maybe we should create our ownAsyncFrom.NormalModuleLoaderShouldYield:module: NonNull<dyn Module>is notSendin RunnerContext: https://github.com/CPunisher/rspack/blob/87945e092d42741490ae4b36d405ce400a375149/crates/rspack_core/src/loader/rspack_loader.rs#L49CompilationRuntimeRequirementInModule,CompilationRuntimeRequirementInChunk,CompilationRuntimeRequirementInTree: It's hard to migrate the related code in https://github.com/CPunisher/rspack/blob/87945e092d42741490ae4b36d405ce400a375149/crates/rspack_core/src/compiler/compilation.rs#L1700block_onin Module trait functions: https://github.com/CPunisher/rspack/blob/87945e092d42741490ae4b36d405ce400a375149/crates/rspack_macros/src/runtime_module.rs#L189, https://github.com/CPunisher/rspack/blob/87945e092d42741490ae4b36d405ce400a375149/crates/rspack_macros/src/runtime_module.rs#L131Checklist