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
perf: introduce parseAstAsync
and parallelize parsing AST
#5202
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov Report
@@ Coverage Diff @@
## master #5202 +/- ##
==========================================
- Coverage 98.82% 98.81% -0.01%
==========================================
Files 231 231
Lines 8850 8861 +11
Branches 2315 2316 +1
==========================================
+ Hits 8746 8756 +10
- Misses 43 44 +1
Partials 61 61
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, that looks really good, especially the numbers. Still, I actually started working into another direction, which I would like to compare with this one first, which would be to move parallelizing out of Rust and instead use a worker in JS.
The advantage would be that this would also parallelize the WASM build, though that may only be relevant to some. It would be more overhead creating the worker(s), though, but considering the potential savings, I would start with a single persistent worker. I hope to finish it tomorrow or the day after, then we can compare.
By the way if we split parseAst
and separately expose parseToBuffer
and convertBufferToProgram
, people could easily implement their own worker approach by putting the first function into a worker. With your approach, of course, everyone would get a parallelized build but there would be no limit to the number of threads.
Awesome!
I guess it is limited by the |
Ok, my worker attempt is at #5211 but I already see that at least the tests run MUCH slower with a worker (but I did not do much profiling yet). It seems that workers are much less light-weight than threads in Rust, which tempts me to parallelize in Rust instead as you implemented and accept that the WASM build will remain somewhat slower for now. |
Ok, to be more precise, the worker approach is much slower for very small builds, as you would have in a test: A build of 40ms now takes 80ms for me. On the other hand, using the "ten times three.js benchmark", the worker approach is still considerably faster for me, around 10%. So it seems, there is just an initial overhead for workers that cannot be ignored. |
Would a different interface make it easier to handle? This interface would work for Vite. let workerRefCount = 0
let worker
const getWorker = () => {
workerRefCount++
return worker || new Worker('/path/to/parseWorker')
}
const stopWorker = () => {
workerRefCount--
if (workerRefCount === 0) {
worker.terminate()
}
}
export async function createAsyncParser() {
const w = getWorker()
const parseAsync = async (
code: string,
allowReturnOutsideFunction: boolean,
_signal?: AbortSignal | undefined | null
) => w.parse(code, allowReturnOutsideFunction);
const stop = () => {
stopWorker()
}
// warn for node if the process existed with non-zero exit code without calling `stop`?
return { parse: parseAsync, stop }
} |
The problem that Rollup is facing is that it does not fully know when it can terminate the worker. Usually, you can throw away the worker after the build phase. However, there are some edge cases where you still need to do parsing during generate phase. But we cannot know beforehand if and how many outputs will be generated. We could also tie it to the |
This PR has been released as part of rollup@4.2.0. You can test it via |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Description
This PR introduces a new function
parseAstAsync
that runs the parse in a different thread by using AsyncTask. There's a more simpler way to execute parallelly by using tokio, but because SWC uses rayon, a different parallelism library, I guess it's better to avoid using that.Running the benchmark in #5182 (actually I used the esbuild repo instead), I got ~850ms / ~12% improvement on my laptop. Putting together with #5201, I got ~1100ms / ~16% improvement.
Benchmark results
rollup 3.29.4
rollup 4.0.2
rollup 4.1.0
parseAstAsync
(~850ms / ~12% improvement)using mimalloc +
parseAstAsync
(~1100ms / ~16% improvement)Specs of my laptop
This change increases the binary size by 0.13MB (3.25MB -> 3.38MB).