-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Non-deterministic module ordering with splitting: true
(affects chunk hashes)
#1075
Comments
Just out of curiosity, is the single-threaded |
Thanks for the detailed report. However, I can't reproduce this on any codebase I have access to, including the Figma which is usually large enough and complex enough to catch most issues. I assume sharing the Framer codebase is not an option, so this might be tricky to debug. I really wish I had access to more large codebases for testing (especially automated tests). A start could be to know for sure which commit caused the issue. You can build esbuild locally using these instructions: #969 (comment). I assume you'd need the I appreciate you helping to solve this. The linker has been relatively stable up until recently, but it's currently undergoing a lot of changes due to the rewrite. I'm hoping to get it to a stable state again soon but with the ordering issues you had previously identified fixed. |
Looks like this reproduces as far back as in v0.5.16 (where code splitting was introduced) 😕 Symptoms are identical – it’s a few modules being moved up or down, and it’s the same modules (I’ve noticed Figuring out if I can provide more information or compose at least some repro. |
Hm, would you be open to pair-debugging this remotely on my machine? This is something that Framer folks are definitely open towards. I’m not sure I’ll be able to produce a working repro for this – all my attempts so far were unsuccessful. If you’re open & have time for this, what’d be the best way to reach out to you to agree on date/time? I’m available this weekend & any day after Apr 12th. I’m located in Europe, but timezone differences should not be an issue. Here’s how I imagine the setup:
|
Have you tried building the non-determinisitic scenario with esbuild run with Go's race detector? |
Right, thank you! Missed your suggestion 👍
Just tested with
There’s no sense checking this given that Just thinking out loud.
|
Groan... nevermind: esbuild/internal/bundler/linker.go Lines 2888 to 2891 in be13300
|
Yes! Maps are used all over the place, and map iteration order in Go is deliberately random. In fact Go's guarantees are stronger than this; map iteration order is actively randomized to try to create bugs:
I'm very aware of this and I always try to use sorted iteration order when relevant. I agree that this is likely the culprit, but finding the spot isn't going to be easy for me because esbuild works fine with everything I run it on (including Figma, which is a huge codebase).
Yes, that could also be the culprit. The specific function is
Huh. One one hand that's not great because it's been around so long. But on the other hand that means it's not a recent regression, which means might be somewhat obscure since it's only coming up now.
Thanks for the offer! Offering your time is very generous. That should definitely let us get to the bottom of this. I'd like to try to investigate this more myself first before taking your time but maybe we can do that this weekend sometime if it's still not figured out by then? |
Any chance the commits above (on master) do anything to fix this? They switch the tie-breaking sort key from the file path to the index of the file in DFS order over all entry points. File paths are supposed to be unique but after a lot of searching, I found one case where there weren't: auto-generated stubs for CSS files that are imported into JS files. These stub files are completely empty so their order shouldn't matter, put it's possible they could still affect the hash. |
Nope :/ I’m still seeing drifts in consecutive builds even with Just emailed you to see how we can connect. |
@iamakulov and I just finished a debugging session for this. Thanks very much for helping to debug this with me! The good news is that it looks like this problem no longer reproduces in version 0.11.5 (the latest version). We believe that this is also related to incorrect handling of I'm adding some notes about this session below, mostly for me and anyone else who is interested: For context: during tree shaking, each file keeps track of which entry points that it is reachable from, as well as the minimum "distance" from any entry point (the number of import edges away from an entry point in the import graph). Code splitting chunks are determined by grouping files by the set of entry points that they are reachable from, and then order of files within a chunk is determined by sorting on the minimum distance entry point. Note that the order within a chunk is different than ESM, which is another problem: #399. I plan to eventually replace all of this code with something entirely different that better matches ESM semantics, which will make this bug irrelevant. But for now this bug still matters while the current legacy code splitting algorithm is still active. The underlying cause of the non-determinism appears to be that the calculation of The function esbuild/internal/bundler/linker.go Lines 2535 to 2545 in 00269f3
But esbuild/internal/bundler/linker.go Lines 2679 to 2688 in 00269f3
I suspect this ended up mattering when certain import graph edges aren't traversed due to Since this is a fixed-point iteration that computes some minimums and sets, the result should be deterministic regardless of traversal order, so traversal order shouldn't matter. The reason why it mattered was that the computation for the minimum was incorrect. The minimum computation needs to be fixed. |
I'm closing this as I believe this should now be fixed. In addition to this no longer reproducing in version 0.11.5, I believe the separation of tree shaking and code splitting into two separate passes in version 0.11.7 should ensure that |
I’ve stumbled upon an issue where the order of modules in the bundle in non-deterministic. (As in, I run the same build twice, and esbuild puts some modules in a different place in the bundle.)
With code splitting, this causes the same chunk to have a different hash on every rebuild. Ultimately, this breaks long-term caching.
I was not able to compose a minimally reproducible example, but here’re my findings that might be relevant:
Multiple entrypoints. This is reproducible on the Framer codebase. Framer has 10 entry points, but the issue occurs even if you comment out some of them (down to 3-4 active ones).
Code splitting. This only happens when
splitting: true
is set.It’s different modules. Depending on build settings, I was able to reproduce the issue with
popmotion
,framer-motion
,@babel/runtime
andreact-router
. Eg:react-router
moved 800 lines higher:However, the module – and the module’s oscillating location – stays the same unless you change entrypoints or externals. So it’s not totally random.
sideEffects: false
might have an effect? While investigating the issue, I tried to remove thesideEffects: false
flag innode_modules/framer-motion/package.json
. This produced a different kind of drift – ie, withsideEffects: false
, modules were reshuffled one way; but withoutsideEffects: false
, they were reshuffled in a different way.However,
@babel/runtime
does not have thesideEffects: false
flag – and it still was moved around in one setup. So the issue is likely about the module order in general, not about a specificpackage.json
flag.Concrete example. Here’s one concrete case from my experiments:
These are import graphs for two consecutive builds with the same codebase and config. The left build was first, the right one was second. For simplicity, these graphs were built only for a single entrypoint (there were 4 or 5 entrypoints in total). (Zoomable graph version.)
In both builds, all leafs and some intermediate nodes have the same hashes because they are identical. However, there’s a single chunk (highlighted with a red border) that reshuffles a few modules:
Diff
This causes other chunks dependent on that chunk to also change their hashes. That’s partly because the import is now different, but partly because a few modules have drifted inside these chunks as well:
Diffs
esbuild config. The esbuild config that reproduces the issue on the Framer codebase looks like this:
I’m happy to provide more info if there’s anything that can help to figure out the source of the issue!
The text was updated successfully, but these errors were encountered: