src,lib: reducing C++ calls of esm legacy main resolve
This PR is related to https://github.com/nodejs/performance/issues/73
Legacy Main Resolve
@anonrig raised that we could benefit from the implementation of legacyMainResolve on the C++ side, so here's the result so far:
confidence improvement accuracy (*)
esm/esm-legacyMainResolve.js resolvedFile='node_modules/non-exist' packageConfigMain='./index.js' packageJsonUrl='node_modules/test/package.json' n=10000 *** 34.86 % ±1.41%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/non-exist' packageConfigMain='' packageJsonUrl='node_modules/test/package.json' n=10000 *** 34.92 % ±1.52%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/test/index.js' packageConfigMain='./index.js' packageJsonUrl='node_modules/test/package.json' n=10000 *** 34.53 % ±1.22%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/test/index.js' packageConfigMain='' packageJsonUrl='node_modules/test/package.json' n=10000 *** 35.39 % ±1.57%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/test/index.json' packageConfigMain='./index.js' packageJsonUrl='node_modules/test/package.json' n=10000 *** 34.78 % ±1.30%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/test/index.json' packageConfigMain='' packageJsonUrl='node_modules/test/package.json' n=10000 *** 35.39 % ±1.15%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/test/index.node' packageConfigMain='./index.js' packageJsonUrl='node_modules/test/package.json' n=10000 *** 34.75 % ±1.43%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/test/index.node' packageConfigMain='' packageJsonUrl='node_modules/test/package.json' n=10000 *** 35.84 % ±1.33%
(**) (***)
esm/esm-legacyMainResolve.js resolvedFile='node_modules/non-exist' packageConfigMain='./index.js' packageJsonUrl='node_modules/test/package.json' n=10000 ±1.88% ±2.45%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/non-exist' packageConfigMain='' packageJsonUrl='node_modules/test/package.json' n=10000 ±2.02% ±2.64%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/test/index.js' packageConfigMain='./index.js' packageJsonUrl='node_modules/test/package.json' n=10000 ±1.62% ±2.12%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/test/index.js' packageConfigMain='' packageJsonUrl='node_modules/test/package.json' n=10000 ±2.09% ±2.74%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/test/index.json' packageConfigMain='./index.js' packageJsonUrl='node_modules/test/package.json' n=10000 ±1.74% ±2.27%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/test/index.json' packageConfigMain='' packageJsonUrl='node_modules/test/package.json' n=10000 ±1.53% ±1.99%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/test/index.node' packageConfigMain='./index.js' packageJsonUrl='node_modules/test/package.json' n=10000 ±1.91% ±2.48%
esm/esm-legacyMainResolve.js resolvedFile='node_modules/test/index.node' packageConfigMain='' packageJsonUrl='node_modules/test/package.json' n=10000 ±1.77% ±2.31%
Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 8 comparisons, you can thus
expect the following amount of false-positive results:
0.40 false positives, when considering a 5% risk acceptance (*, **, ***),
0.08 false positives, when considering a 1% risk acceptance (**, ***),
0.01 false positives, when considering a 0.1% risk acceptance (***)
To be able to run the benchmark, first build the
old
node using this branch.
About the benchmark, I needed to create another benchmark just for this function to be able to see the improvements since the results of esm-defaultResolver.js was not reflecting the real impact of this change.
Potential optimizations
legacyMainResolve
also calls fileURLToPath
and didn't have any version on the C++ side, so I needed to rewrite this function.
I haven't benchmarked the JS version yet, but I think we can add some benchmarks and see if we can replace the JS version with the C++ version, I've tried to make the code as easy as possible to support exposing on the JS side in the future.
Tasks
-
Rewrite legacyMainResolve -
Rewrite fileURLToPath -
Add benchmark for legacyMainResolve -
Add tests for legacyMainResolve
Acknowledgments
Huge thanks to @anonrig for the support and early review of this code and @RafaelGSS for the hints about the benchmark and permission model.