Fix non-deterministic builds between project directories #8869
Conversation
Benchmark ResultsKitchen Sink ✅
Timings
Cold BundlesNo bundle changes detected. Cached BundlesNo bundle changes detected. React HackerNews ✅
Timings
Cold Bundles
Cached Bundles
AtlasKit Editor ✅
Timings
Cold Bundles
Cached Bundles
Three.js ✅
Timings
Cold BundlesNo bundle changes detected. Cached Bundles
|
| }), | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Wonder if you could use BitSet for bundle.assets here? Then when you do bundle.assets.add it would just be setting a bit corresponding to the position within assets. When we take the ideal graph and convert it back to the legacy graph we could iterate in asset order (as BitSet#values() does).
There was a problem hiding this comment.
Funny you mention this because I considered that approach as well but was worried this iteration cost might not be worth it. You've inspired me to try it though. Will report back.
There was a problem hiding this comment.
@devongovett Just realised it'd be hard to use the BitSet here as we need to create bundles before we know the full list of assets as we add them during the traverse.
There was a problem hiding this comment.
You could create the bitset earlier maybe
There was a problem hiding this comment.
I could, but I'd have to add a second full asset traverse to collect the list of all assets before starting the main traverse. Unless there's some other cheap way to get the list of all assets?
There was a problem hiding this comment.
After migrating all asset sets to BitSets I actually saw a drop in perf. This is due to the values() extraction being too expensive. I think there's potential to use BitSet more widely in the bundler but it would require a larger rethink to optimise around its benefits.
tl;dr let's just sort for now.
Co-authored-by: Devon Govett <devongovett@gmail.com>
↪️ Pull Request
We noticed we were getting different build outputs when running builds of the same commit in different directories. This PR resolves the two issues we were running into.
Firstly, the JS transformer adds hashed import specifiers to require calls but the hash is partly seeded from the absolute filepath. This may get cleaned up afterwards, but it's possible the differing hashes can cause the files to have slightly different output sizes which has downstream effects in the bundling phase. I also think file building should be deterministic in general anyway as it will make cross system caching feasible in the long term.
Secondly, when shared bundles are deleted and merged into the entry, the assets of the entry bundle are now likely out of order. This was fixed by sorting the assets of the modified bundles. This issue may have simply exacerbated the first one from a project root perspective.
🚨 Test instructions
Existing tests should be sufficient for the transformer change. Not sure how to test the shared bundle issue?
✔️ PR Todo