node-resolve doesn't cache lookups

[keithamus]

If node-resolve has found a non-relative module - i.e. one that exists in side node_modules, it should cache this lookups - to save time doing subsequent re-lookups, to avoid hitting the disk again.

I am discovering this because one of my modules takes 10ms to resolve using node-resolve (node-resolve needs to read the package.json to determine where the module exists), and this module is used 100 times in the codebase, currently it takes a second of cumulative lookups for this module, but it should be able to hit a cache so only the first hit takes 10ms while subsequent hits should only be 1-2ms at worst.

Happy to submit a PR if you feel this is correct 😄

[keithamus]

Just to give some more context; on a single build of one of the smaller modules, on the codebase I'm on, I instrumented readFile just to see how many times a package.json file was read (rather than just hit from a cache) and literally every duplicate file read came from browser-resolve, hitting some of my package.json files hundreds of times for one build. Some stats on how many duplicate reads happened:

  '%root/app/assets/package.json': { cnt: 139, min: '0.02', max: '0.16', avg: '0.03', tot: '4.46' },
  '%root/app/package.json': { cnt: 139, min: '0.02', max: '0.12', avg: '0.03', tot: '4.16' },
  '%root/package.json': { cnt: 739, min: '0.06', max: '0.26', avg: '0.08', tot: '57.96' },
  '%root/app/assets/modules/package.json': { cnt: 138, min: '0.02', max: '0.21', avg: '0.04', tot: '5.23' },
  '%root/node_modules/module-1/package.json': { cnt: 18, min: '0.06', max: '0.12', avg: '0.07', tot: '1.24' },
  '%root/node_modules/module-2/package.json': { cnt: 18, min: '0.06', max: '0.13', avg: '0.07', tot: '1.34' },
  '%root/node_modules/module-3/package.json': { cnt: 4, min: '0.06', max: '0.07', avg: '0.06', tot: '0.25' },
  '%root/node_modules/module-4/package.json': { cnt: 7, min: '0.09', max: '0.31', avg: '0.15', tot: '1.08' },
  '%root/node_modules/module-5/package.json': { cnt: 2, min: '0.06', max: '0.19', avg: '0.12', tot: '0.25' },
  '%root/node_modules/module-6/package.json': { cnt: 14, min: '0.06', max: '0.15', avg: '0.08', tot: '1.07' }

(I've had to strip some of the path info because the modules and pathnames are sensitive info)

Now the above also times the disk-reads for these files, which are relatively short because I'm using an SSD, and reading just the package.json files isn't the slowest part of the resolve operation (for example, the resolved file is also read from disk to see if it looks like an es6 module)

Also, some of those package.json files simply dont exist - despite being read 139 times each, none of app/assets/package.json, app/package.json or app/assets/modules/package.json exist on the filesystem.

As you can see during the rollup build; browser-resolve read my package.json 738 times! phase-1 of the rollup build process (which is just resolving the module ids) takes ~2900ms on my machine. module-1, module-2 and module-6 in the above stats also took the longest to resolve (with module-2 taking an average of 20ms for this build for the whole resolve call, including reading package.json from disk). So in other words module-2 was manually resolved 18 times, each time taking ~20ms for a total of 360ms of this particular build. Between module-1, module-2 and module-6 (all of which, are not only called the most but also take the longest to resolve) we're spending 680ms just re-resolving the same module.

[lukastaegert]

Hi @keithamus, without checking this myself (your analysis looks quite thorough 👍), yes, a PR would be very much welcome here!

[keithamus]

I haven't forgotten about this! I'll be picking this up next week or the week after.

[pieterbeulque]

Hi @keithamus, have you started working on this? I'd be glad to help out.

[keithamus]

Not yet. Probably going to have to pick it up in the new year now. Feel free to work on it in the meantime!

[keithamus]

I've made the first of a couple of PRs, #126. This cached disk stats and reads from node-resolve. Takes the number of these calls down considerably.

[dima-takoy-zz]

Currently, plugin caches files (https://github.com/rollup/rollup-plugin-node-resolve/blob/master/src/index.js#L15, https://github.com/rollup/rollup-plugin-node-resolve/blob/master/src/index.js#L26).

Can it be closed now? Or we waiting for some enhancements?

[keithamus]

Oh yeah, we can close this now. Thanks @mecurc!