RFC: V3: Oxc resolver

[alshdavid]

Maintaining our own Nodejs resolver is quite difficult, error prone and increases the maintenance/testing surface of Atlapack. This RFC is about replacing our native resolver with an oxc based resolver and supplies a reference for testing.

Using our benchmarks, oxc is 10% faster (though the actual plugin is doing less so YMMV), would like to test this against JFE to see if it resolves the tsconfig issues we are seeing

To use this just add @atlaspack/resolver-oxc to the .parcelrc file

[devongovett]

FYI, I made Parcel's resolver a lot faster recently (basically rewrote the whole cache): parcel-bundler/parcel#9984

Benchmarked a bunch of resolver crates including OXC: https://github.com/parcel-bundler/parcel/blob/v2/crates/parcel-resolver/benches/benchmark.rs

With caching:

cached/parcel_resolver  time:   [1.8614 µs 1.8661 µs 1.8716 µs]
cached/oxc_resolver     time:   [6.4001 µs 6.4171 µs 6.4362 µs]
cached/rspack_resolver  time:   [6.3623 µs 6.3738 µs 6.3856 µs]
cached/deno             time:   [81.661 µs 82.007 µs 82.467 µs]

Without caching (creating a fresh cache on each iteration):

uncached/parcel_resolver time:   [109.29 µs 109.70 µs 110.13 µs]
uncached/oxc_resolver    time:   [162.42 µs 163.24 µs 164.28 µs]
uncached/rspack_resolver time:   [165.43 µs 166.10 µs 167.01 µs]
uncached/deno            time:   [157.49 µs 157.66 µs 157.88 µs]

Published it on crates.io as well in case you want to use it. https://docs.rs/parcel-resolver/

[yyx990803]

I ran the benchmark myself and the difference doesn't seem to be this significant

Edit: noticed we need to have the node_modules populated before running the bench. Impressive work.

We haven't touched the oxc resolver for a while, so we can surely take a look to see where we can improve.

[yamadapc]

I think it does make sense to maintain a resolver because we would rather have no resolver plugins.

Instead, we'd add the few patches we are currently using on resolver plugins to the main rust code.

---

@devongovett we can't use parcel resolver because it depends on dashmap which we have now validated was causing segmentation faults on our developers machines. There were hundreds of native crashes every day, which are completely fixed by #10.

That is, there is some unsoundness in the dashmap crate which leads to memory corruption and then crashes under a certain workload, which we started to hit. Developers were unable to start the server and make a few builds without a crash prior to this change.

---

Regarding benchmark, while it is good to micro-benchmark, it's better to measure a real workload.

We have an internal benchmark, which replays all imports in our application.

On that benchmark, the current master branch of this repository performs as well as the parcel master branch before cache optimisation ; the cache optimisation changes made parcel resolution slower by a non-negligible amount (10%+).

Most importantly, the absolute times to resolve all imports in the repository are very low when they are executed concurrently (a few seconds to resolve all the imports with multi-threading).

I think that if resolution is a build time bottle-neck today, it is only because the builds are based on crawling the module tree, and it'll be much more efficient to crawl the directory tree directly to parse and discover imports, then run resolution as a second time.

---

Note that despite around 30% slower resolver (at the moment) in the master branch, and previously we had a 3x slower resolver than parcel master #156, the actual build times for this repository are 2x faster to build jira than what is in mainline parcel due to a few optimisations on packaging/optimisation phases.

There is currently some degradation in perf. of the master branch resolver, when running new experimental code for building the asset graph in rust. This might be related to removal of Cow on certain resolver parts. I am unconvinced it is a performance bottle-neck, but there are other fixes to it.

[yamadapc]

Let's talk about this internally.

[devongovett]

FWIW, oxc_resolver also uses dashmap. Dashmap v6 was recently released as well. Maybe that fixed some issues you saw.

[yamadapc]

I was hoping that was the case too, but I had looked at the diff between those dashmap versions and there did not seem to be significant changes that could fix the crashes.

What we have now (thread local caches) performs really close to the concurrent dashmap but has much less surface area for unsoundness and is easier to understand.

It'd be nice to review this at some point but we were having serious unreliability issues related to crashes (they were common, and certain engineers would experience crashes on every single run of certain builds).

[alshdavid]

Will continue this discussion after I get back