nodejs / performance

Node.js team focusing on performance
MIT License
371 stars 7 forks source link

Fast path for CommonJS on Module._compile #139

Open H4ad opened 7 months ago

H4ad commented 7 months ago

In theory, we can move the load of content of a module to C++ when:

Since the content goes directly to the C++ without modification:


Also, I don't have any idea if worth or is viable but maybe we could use StreamedSource instead of Source, in this way, we probably can save some memory and load the script much faster.

Let me know what you think, this is something that was in my mind for a while and I just wanna to share it with more people to understand if is possible or not.

Uzlopak commented 7 months ago

Pinging @nodejs/loaders

GeoffreyBooth commented 7 months ago

In general we've been treating the CommonJS loader as legacy that is frozen and not to be updated, because we don't have the resources to debug any issues that may result from changes there. No one on the current loaders/modules team has worked with the CommonJS loader, as it hasn't had updates (other than parts shared with the ESM loader) in years.

The ESM loader is much more ripe for optimizations, if I can encourage you to focus there.

joyeecheung commented 6 months ago

I don't think we've ever agreed that CommonJS loader is a legacy. It is frozen, but only because of the impact caused by compatibility. Many CLI tools (e.g. eslint, npm and tsc) still choose to bundle their sources as a CommonJS bundle (with parhaps one or more CommonJS entry points) to speed up start up time, so optimizations of the CommonJS loader would benefit the ecosystem through these popular apps that still use CommonJS.

No one on the current loaders/modules team has worked with the CommonJS loader, as it hasn't had updates (other than parts shared with the ESM loader) in years.

I don't think this is true, looking at the git log of lib/internal/modules/cjs/ I see >10 updates for SEA, policy and the permission model in just the past year (and in the year before, updates for --watch), all from people outside of the loaders/modules team. I think the state is just that the modules/loaders team do not possess any more expertise on the CommonJS loader than other contributors, since those teams were created specifically for ESM and the CommonJS loader is not in their scope. The CommonJS loader is a long-time resident that people are just more familiar with than the ESM loader, it's also much easier to hack on for contributors not familiar with any loader due to its smaller size and lower complexity IMO (its inner working is also widely documented on the Internet via various deep-dive blog posts).

arcanis commented 6 months ago

In theory, we can move the load of content of a module to C++ when:

H4ad commented 6 months ago

@arcanis You mean if we move the loading of the content to the C++, we will not be able to patch fs to support virtual filesystems?

If this is true, I think we already broke the support for virtual filesystems by moving the package reader to C++ (during module resolution if I remember correctly).

arcanis commented 6 months ago

If this is true, I think we already broke the support for virtual filesystems by moving the package reader to C++ (during module resolution if I remember correctly).

Yes, and it had to be fixed.

It's fine to use faster primitives, but there should be a way for virtual filesystem to still affect them. Ideally that would be through a blessed API, perhaps similar to the loader hooks, and failing that through discouraged access points.