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

Delay loading a set of ids until all other files have been processed #4985

Open
patak-dev opened this issue May 9, 2023 · 11 comments
Open

Comments

@patak-dev
Copy link
Contributor

Feature Use Case

I don't know if this will require a new feature in Rollup, it may be that we aren't aware of a good way to implement this feature in Vite.

We have an experimental feature to pre-bundle dependencies during build time using esbuild. We do this during dev because of CJS interop (we don't use rollup plugin CommonJS) and we want to reduce the number of files we sent to the browser (one file for all of lodash). During build, pre-bundling would allow us to reduce the difference between dev and build and hopefully speed up build times because deps pre-bundling is cached. It could also potentially help us with memory consumption.

To be able to implement prebundling, we first need a complete list of all entry points to dependencies that should be prebundled. Then we do a single esbuild bundling with all these entry points. To get the list, we need to wait for every file that isn't an entry point to be optimized to be processed. We have a scheme in place where we register all non-optimized ids during resolveId and then call this.load() on them. On the load hook we wait for all these load events to finish, then we optimize with esbuild, and finally resolve the loading of all the optimized deps.

The issue is that each awaited optimized dep will count towards maxParallelFileOps, so now users need to bump this option for the scheme to work.

We tried alternative options, like registering during resolveId (using a timeInterval to avoid an early exit), and then marking the id as done during transform with post order. The effect is the same. Here is another option from @sun0day, but I feel we are missing a simpler alternative.

Feature Proposal

Is there a way to avoid starting loading of optimized deps so they don't count towards maxParallelFileOps? Maybe a new feature is needed here or there is something we are currently missing. Sorry for the semi-support disguised as a feature request in that case.

@lukastaegert
Copy link
Member

Maybe we could use the object form of hooks to mark hooks that should ignore that file operation limit? We would still need to think how this should work internally, but would this solve the issue? E.g. ignoreMaxParallelFileOps: boolean as a property of the hook similar to order?

@patak-dev
Copy link
Contributor Author

That could work, yes. I still feel we may be missing a simpler strategy (more thinking on Vite working with pre-bundling as default during build). If Rollup would add a specific option for this case, maybe it could already be an option that makes the whole scheme work without hacks.

For example, during resolveId, the optimized dep resolves to an object like { id: "...", delayLoading: true } and rollup would avoid calling load on these ids until every other id has been resolved, loaded, and transformed (the equivalent to this.load called on all of the ids). This is what we're trying to implement conceptually at least.

If not, we are forced to do one of the following:

  • Call this.resolve(skipSelf) + this.load(resolved) on resolveId for every id that isn't optimized
  • or, call this.resolve(skipSelf) on resolveId, register it, use setTimeinterval to trick rollup into not exiting early, then on transform post mark the id as done
  • or something else, call this.load for every id on the load hook?
    Maybe it isn't a problem for rollup though, I'm just a bit unease that we need to keep track of every transform end of almost every id.

@lukastaegert
Copy link
Member

the optimized dep resolves to an object like { id: "...", delayLoading: true }

Ah, that is actually a great idea. Some time ago I had the idea in my head to have something like "delayed dependencies", and/or a special hook that runs when the module loader is "idle" to be able to emit additional entries that rely on the current module graph. But this suggestion is actually simpler.

So let me try to flesh this out a little more.

So assume a plugin resolves to foo with delayLoading: true. Then Rollup will store this id (and the the ResolveId object) for later.

When the module loader is done resolving and loading everything it can and would run idle, it then picks the first delayed id. The next time it runs idle it loads the second delayed id and so on.

What happens if some other import is resolved to an id that is marked as "delayed" but without delayLoading? I see two possibilities:

  1. It is removed from the delayed ids and loaded right away, or
  2. We say delaying is more important, and it remains delayed?

@patak-dev
Copy link
Contributor Author

I think I would go with 2., if someone added delayLoading I would expect that to be respected.

When the module loader is done resolving and loading everything it can and would run idle, it then picks the first delayed id. The next time it runs idle it loads the second delayed id and so on.

Another option would be that all the ids marked with delayLoading are grouped until things run idle. And at that point, all of them are loaded (potentially in parallel). And the process begins again, every new delayLoading id will wait for the next idle. For us at least, this would be more efficient as we could load all the files in parallel. The delayed deps don't need to wait for the other delayed deps. But any of the two schemes would work for us.

A bit more complexity, just to put everything on the table. During build time, Vite could find worker entries. These are bundled in a separate rollup process when they are found (with a different set of plugins). But they share the same optimized deps processing and cache. Right now, we keep track of the ids that are being processed on all these rollup calls. If we get a delayLoading, we would also use it in the worker rollups but I think we'll need a bit of hacking. I think we'll need to wait for all the rollup processes to be iddle by counting the processes and awaiting the optimized deps load once called (sounds simpler than what we do now).

@lukastaegert
Copy link
Member

Another option would be that all the ids marked with delayLoading are grouped until things run idle. And at that point, all of them are loaded (potentially in parallel)

Yes, I agree that would be even better.

As I see it, it should be possible to implement something along these lines. If one from the Vite team wants to have a stab at this, I would be glad to support, otherwise it may need some time before I can look into this.

@bluwy
Copy link
Contributor

bluwy commented Jun 7, 2023

Dropping a related link that this feature would help #4294.

@patak-dev
Copy link
Contributor Author

I think delayLoading would help #4294 if we don't make it a boolean, no? This is something I was thinking while working on the feature, if we document delayLoading in rollup, there surely end up being other projects that would find it useful and then start using it. But for Vite to work, this feature should only be used by Vite.

Some options:
a. delayLoading: number, and we load each group of ids with the same number starting by lower first.
b. we keep delayLoading: boolean and document that if other projects use this feature, missing dependencies inside these modules will not be discovered.

I'm leaning towards b.

@bluwy
Copy link
Contributor

bluwy commented Jun 7, 2023

Yeah I think b is good enough. If they really need to wait for delayed modules to be loaded too, they can continue using the trick. The added benefit with this feature is that it won't be blocked on maxParallelFileOps anymore.

@ArnaudBarre
Copy link

This need is also a big requirement to have a clean utility based CSS plugin. For now I've seen to option:

I went with the second option because the number of hacks to support esbuild that doesn't have a "generate chunks phase" was too high.

Given that Tailwind 4 will provide a Vite plugin, this would really have a big impact if something to solve this was available in the Rollup API!

@patak-dev
Copy link
Contributor Author

I have implemented this locally for rollup using the delayLoading: boolean API. But I'm no longer convinced that this API should be added. The original use case in Vite (delay the loading of dependencies until all user code has been processed) is no longer needed. Your use case sounds legit though @ArnaudBarre, but at the same time shows that the proposal wasn't enough. If it isn't only Vite core the one that would like to use this feature, then we need something more complex, and it isn't easy to define. delayLoading: number will get us into a competition against each other. If we add something, we probably want something like a dag of delayed loading groups and users could decide what needs to wait what.

@ArnaudBarre
Copy link

Yeah the need is slightly different, the need to be able to know when all the load from a certain type is done. But this is tricky because it can easily introduce dead locks

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

No branches or pull requests

4 participants