module: compute module.paths lazily #51611
Conversation
Make module.paths a getter which computes the property when it's requested. The paths property is entirely deterministically computed from the path property, so it's not necessary to keep the relatively large paths array in memory. On an internal application, this reduces the used heap by around 5.5%. Furthermore (although it was not a goal of this PR) it speeds up the execution time of `vite --version` by ~2% according to hyperfine. A similar speed improvement is observed on the internal application. Notably, this introduces two minor changes which may be considered breaking. The obvious one is that `paths` is now a getter/setter and not a property. The other one is more subtle: the lazy computation means that if a user changes `mod.path = X` and then later invokes `mod.paths`, the results will reflect the updated path as opposed to the current behavior where it would reflect the original path. One way to fix the latter would be to force capturing `paths` before `path` is changed, I'm open to doing that (or some cleaner approach?) if the reviewers deem it necessary.
|
Review requested:
|
nodejs-github-bot
added
esm
| const lazyPathsGetter = { | ||
| __proto__: null, | ||
| enumerable: true, | ||
| get() { return this[kPaths] ?? Module._nodeModulePaths(this.path); }, |
There was a problem hiding this comment.
I think this could also just replace this.paths with a value property once computed so subsequent accesses are not invoking a getter again & the property type change is less observable from the user land.
aduh95
added
needs-citgm
|
Added the dont-land labels for LTS because of the potential for breakage mentioned in the OP. |
|
I would still like to see this happening...@kvakil are you still working on this? |
|
This needs a rebase. |
|
This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open. |
|
Closing this because it has stalled. Feel free to reopen if this issue/PR is still relevant, or to ping the collaborator who labelled it stalled if you have any questions. |
Make module.paths a getter which computes the property when it's
requested. The paths property is entirely deterministically computed
from the path property, and so it's not necessary to keep the relatively
large paths array in memory.
On an internal application, this reduces the used heap by around
5.5%. Furthermore (although it was not a goal of this PR) it speeds
up the execution time of
vite --versionby ~2% according tohyperfine. A similar speed improvement is observed on the internal
application.
Notably, this introduces two minor changes which may be considered
breaking. The obvious one is that
pathsis now a getter/setterand not a property. The other one is more subtle: the lazy computation
means that if a user changes
mod.path = Xand then later invokesmod.paths, the results will reflect the updated path as opposedto the current behavior where it would reflect the original path.
One way to fix the latter would be to force capturing
pathsbeforepathis changed, I'm open to doing that (or some cleaner approach?)if the reviewers deem it necessary.