Module resolution

[sheplu]

Following the tweet by Marvin - and the subsequent article we should see how we can take that feedback into account and see how we can improve the performances around module resolution

• stacktrace / throwIfNoEntry in fs.stat
• cache when accessing file
• file extension

[marvinhagemeister]

FYI: It looks like only fs.statSync and fs.lstatSync support the throwIfNoEntry option. Can't find such a thing for the async variants.

[joyeecheung]

In the meeting we mentioned it might be possible to implement the sync stat functions with fast calls. The stats are assembled from a typed array, which can be updated just fine in fast calls without triggering allocations.

[joyeecheung]

It's also mentioned that fs.existsSync can be used, although it now still does an try-catch because we have a funny behavior that is pending to be deprecated: https://github.com/nodejs/node/blob/762a3a8ad925d56a12b43e0e7f7c811e93097784/lib/fs.js#L272-L277 technically I think we can still avoid that even without breaking it if we return false instead of just throwing in the validation helpers.

[tony-go]

I'll take a look at it in a couple of days

[tony-go]

In the meeting we mentioned it might be possible to implement the sync stat functions with fast calls. The stats are assembled from a typed array, which can be updated just fine in fast calls without triggering allocations.

Hey @joyeecheung 👋🏼

Do you mean using uv_fs_statfs without the SyncCall wrapper?

[debadree25]

In the meeting we mentioned it might be possible to implement the sync stat functions with fast calls. The stats are assembled from a typed array, which can be updated just fine in fast calls without triggering allocations.

I attempted something similar here https://github.com/nodejs/node/pull/46345 I think this could be made slightly faster using internalBinding('stat') but haven't found any consensus yet on whether such a API is useful @joyeecheung @tony-go

[anonrig]

On hold, until off-thread loader pull request is merged.

[kvakil]

stacktrace / throwIfNoEntry in fs.stat

FWIW I've seen the particular "fs.stat being slow due to captureLargerStackTrace" issue before. For the particular case I was looking at, it was actually caused by the deoptimizer (which is needed for the stack unwinding) forcing some garbage collection step to complete (v8 bug).

I'm not sure to what extent that was the cause of the issues that @marvinhagemeister saw, but at least in some cases it should be better soon.

[marvinhagemeister]

@kvakil I saw it in the context of jest and eslint. Common configuration of these two projects involve adding their own compilation layer which means they do module resolution themselves to support tsconfig.json paths and similar things. It's these custom resolutions that are usually node tuned for perf. While looking at that I noticed that it's way better in node as is, but still with room for improvement. The scenarios I looked at loaded a very high number of files, mostly coming from a plethora of npm dependencies.