perf(core): check files before interacting with them

[FauxFaux]

Q	A
Fixed Issues?
Patch: Bug Fix?	Yes
Major: Breaking Change?	No
Minor: New Feature?	No
Tests Added + Pass?	Yes
Documentation PR Link
Any Dependency Changes?	No
License	MIT

What this does

babel is relevantly slower than ts-jest at transpiling our codebase, after caching. A profiler suggests these file APIs are slow. Use existsSync instead, which is quite a bit faster.

This is a problem I've seen before, and potentially raised as a bug here before, but am currently unable to find. 🤷

Benchmarks

require('./app') (inside jest, which has other overheads) goes from 6.5s±0.5s to 5.0s±0.1s, which is >20% faster. For reference, ts-jest's cache can do the same operation in 4.3s±0.1s.

Node 14, Ubuntu, linux 5.8, desktop i7-8700, nvme, btrfs. @babel/preset-env, @babel/preset-typescript. Around 2,000 source files, around 44,000 files inside node_modules appear to be referenced, according to strace -ff -yy -estatx,openat node =jest --runInBand require-stuff.

Semantic change

This is technically a semantic change, if you previously had files that you couldn't read, you will now see an error when trying to read them (after this function), instead of them being silently ignored. I think this is better behaviour. If we don't agree, maybe we could check existsSync then accessSync.

Profiler output

  ticks parent  name
   5734   73.6%  /usr/bin/node
   4109   71.7%    /usr/bin/node
    619   15.1%      LazyCompile: *uvException internal/errors.js:402:21
    205   33.1%        LazyCompile: ~handleErrorFromBinding internal/fs/utils.js:303:32
    167   81.5%          LazyCompile: ~accessSync fs.js:214:20
    152   91.0%            LazyCompile: *sync node_modules/@babel/core/lib/gensync-utils/fs.js:18:7
     15    9.0%            LazyCompile: ~sync node_modules/@babel/core/lib/gensync-utils/fs.js:18:7
     38   18.5%          LazyCompile: ~statSync fs.js:1081:18
     30   78.9%            LazyCompile: ~fileMtime node_modules/@babel/core/lib/config/files/utils.js:30:19
      8   21.1%            LazyCompile: ~<anonymous> node_modules/graceful-fs/polyfills.js:306:21
    170   27.5%        LazyCompile: *fileMtime node_modules/@babel/core/lib/config/files/utils.js:30:19
    141   82.9%          LazyCompile: *<anonymous> node_modules/@babel/core/lib/config/files/utils.js:20:37
    136   96.5%            LazyCompile: *sync node_modules/@babel/core/lib/gensync-utils/async.js:26:9
      5    3.5%            LazyCompile: ~using node_modules/@babel/core/lib/config/caching.js:216:8
     29   17.1%          LazyCompile: ~<anonymous> node_modules/@babel/core/lib/config/files/utils.js:20:37
     29  100.0%            LazyCompile: *sync node_modules/@babel/core/lib/gensync-utils/async.js:26:9

I believe the uvException is explaining that it's expensive to throw and catch and exception, even if you ignore the caught value.

[babel-bot]

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/45558/

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 429e2e6:

Sandbox	Source
babel-repl-custom-plugin	Configuration
babel-plugin-multi-config	Configuration

[nicolo-ribaudo]

I think in the past we already used existsSync, but then switched to accessSync because fs.exists (the async version) is deprecated and we got confused.

[FauxFaux]

That's good to know. I was worried that this was there for a real reason; couldn't see anything in the git history.

[nicolo-ribaudo]

This is technically a semantic change, if you previously had files that you couldn't read, you will now see an error when trying to read them (after this function), instead of them being silently ignored. I think this is better behaviour.

That's a good change imo. If someone is passing a file to Babel and Babel cannot read it, it's better to throw an explicit error than to silently not do what the user is asking Babel to do.

[nicolo-ribaudo]

Thanks!

[JLHwung]

For reference, both fs.accessSync and fs.existsSync call binding.access under the hodd:

https://github.com/nodejs/node/blob/fa6d084dcbe1bf5e1b7d9aee2e02758b84c0aade/lib/fs.js#L230

https://github.com/nodejs/node/blob/fa6d084dcbe1bf5e1b7d9aee2e02758b84c0aade/lib/fs.js#L268

but existsSync does not handle the thrown uvException, instead it just checks the errno so it is faster than accessSync.