-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Thoughts on thread-unsafe plugin usage patterns #5
Comments
This is an incredible thorough research! It is really useful too for the Environment API work.
It would be good if we start moving what we can in Vite core to use it too. One issue here is that the current implementation during dev is buggy (for example, meta is shared between client and ssr). We'll work on this for Vite 6 so we can recommend this pattern to the ecosystem.
We are now exposing the current const countResolveIdCallsPlugin = () => {
const cache = new WeakMap<Environment, Data>()
return {
name: 'count-resolve-id-calls',
buildStart() {
cache.set(this.environment, initData())
},
resolveId() {
cache.get(this.environment).ids++
}
buildEnd() {
console.log(this.environment, cache.get(this.environment).ids)
}
}
} The idea here is that this cache is only for the plugin to consume as @sapphi-red described in the doc, so it is thread safe. Sometimes the plugins that need to share a cache are also a small group of related plugins, separated because it is easier to implement as separate plugins. I think thinking on running groups of plugins in the same thread instead of a thread per plugin is good. Maybe plugins could tag themselves with a group name, and all with the same name are run together so they can easily share their internal caches?
I'm lately thinking that an option is also for
We'll continue to open discussions like the |
Yes - I thought of this too. In a way they are meant to be run in the same thread like sub-plugins of a bigger one.
I'm thinking more about consistency and avoiding duplication, since Rolldown already implements logic that loads and applies plugins - if we implement the container in Vite, how can we leverage the underlying parallelization logic implemented by Rolldown? |
Yes, you're right here. Anything that could be shared with build and exposed to Vite would be great. Some of the Vite specific things:
So yes, maybe this should be in Rolldown and I'm just not seeing where to draw the line right now, especially for things like the dev ModuleGraph. If we move that to rolldown, it may be an opportunity to see if we could have make the module graph in dev and the module structures in build more aligned. I think there were projects running a dev server during build just to get the module graph populated and use it to do optimizations (I think Astro did this at one point, but they moved away from it). |
Here's the places qwik uses
It seems qwik uses to communicate between qwik and qwik city and qwik city's internal plugins and most of them are (1).
Ah, yeah, given that rolldown can process the config file itself, maybe Rolldown can do some variable inlining so that users can use variables outside the function scope in some simple cases.
Many plugins expect A similar issue exists with |
I guess it should be fine for plugins I put in the examples. In those plugins, it will be only used by client modules. That said, It'd be nice to fix it.
That's an interesting idea. I was only thinking about running each plugin in all threads like the figure below. The problem with that is if a plugin that processes many files were slow, the perf won't improve.
Actually, counting the number of resolve id calls matches (A5). Because I was thinking of running a single plugin in all threads, that each WeakMap only has the information of that thread and needs to aggregate the number across all threads to get the total count. |
I checked how |
I also checked whether const plugins = [
{
name: 'worker1',
renderChunk(code, context) {
const meta = context.getModuleInfo('foo').meta
meta.bar = name // set thread id
}
},
{
name: 'worker2',
renderChunk(code, context) {
const meta = context.getModuleInfo('foo').meta
return '' + meta.bar // random value is returned because all threads run in parallel
}
}
] |
The |
|
Context: https://github.com/sapphi-red/parallel-js-plugin-experiment/blob/main/research-for-design.md
It's great that many common caes can be solved with
module.meta
!Agree that we should make
chunk.meta
work likemodule.meta
and maybe also need something likecurrentBuild.meta
(exposed via pluginthis
context maybe?), essentially officially supported mechanisms for storing thread-safe data.Regarding
plugin-vue
- we do have the option to re-design plugin-vue to be more threading-friendly. But for now, cache-miss is not the end of the world.We don't need to worry about
@rollup/plugin-node-resolve
or@rollup/plugin-commonjs
- these are built-in features of Rolldown so they are not going to be needed anyway. Similar for@rollup/plugin-replace
,vite:esbuild
,vite:esbuild-transpile
etc.This seems to be the biggest blocker. I think we may need to identify cases of:
api
for exposing / mutating serializable optionsapi
for injecting functions (e.g.sveltePreprocess
)(1) Seems to be straightforward, but (2) is going to be problematic. Can you link to some examples of how Qwik uses this pattern?
I think we'll have to enforce serializable config formats (I believe both PostCSS and Babel supports that, i.e. specifying plugins via package names so the loading of the plugin is deferred to be controlled by the tools) - luckily should be less of an issue when users move towards Lightning CSS and Oxc for the transforms?
Re simple functions in configs - we might be able to serialize that? But maybe this will just be a hard limitation.
vite-plugin-inspect
may have to be implemented as a built-in. This should not be common.Not sure if I understand the issue with (A10a) - couldn't Rolldown just ignore those hooks like Rollup does? For example,
configResolved
is called by Vite during config resolution and don't really need to be parallelized.My general thought on Vite internal plugins is that Rolldown will expose a "plugin container" API - initially this can be a Node.js API where Vite core use to replace the current
PluginContainer
implementation, but at a later stage we introduce a small Rust core in Vite which uses Rolldown as a dependency - thePluginContainer
will then be used via Rust APIs and have as many internal plugins Rustified as possible.We will need input from @patak-dev and @sheremet-va on this.
The text was updated successfully, but these errors were encountered: