Skip to content
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

Try to make CI deterministic #4138

Closed
wants to merge 5 commits into from

Conversation

RunDevelopment
Copy link
Contributor

There currently is an issue that the CI gets a different result for raw.rs. I suspect that this is because the code gen iterates a hashmap somewhere without sorting by key first.

After a quick search, I found 2 such occurrences, but I don't know whether they are the cause of the issue. I used crate::sorted_iter to sort the iter, just like in other places.

With this, all for loops in crates\cli-support\src\js\mod.rs have a deterministic iteration order. However, this might not be enough. If some vector iterated in crates\cli-support\src\js\mod.rs was created from iterating a hashmap directly, the output will still not be deterministic.

@RunDevelopment
Copy link
Contributor Author

Okay, so after replacing all Hash{Map,Set}s in cli-support with BTree, the problem persists. So it is not caused by any of those data structures (at least, directly).

@RunDevelopment
Copy link
Contributor Author

And that was all Hash{Map,Set}s in the entire repo replaced. The issue persists. So it is not caused by any of the hashmaps in this repo.

The issue might step from external deps or cfgs. Not sure.

@RunDevelopment
Copy link
Contributor Author

Alright, I analyzed the code and narrow it down to all changes in raw.rs being caused by the JS code for Intrisic::ObjectDropRef and Intrisic::Throw being generated in the "wrong" order. (Well, order doesn't matter, it just needs to be the same order everywhere.)

To be specific:

  • Intrisic::ObjectDropRef causes Instruction::ExternrefLoadOwned, which in turn generates all globals from const heap to function takeObject(idx).
  • Intrisic::Throw causes Instruction::MemoryToString, which in turn generates all globals from const lTextDecoder to function getStringFromWasm0(ptr, len).

This explains why the order of export function __wbindgen_object_drop_ref and export function __wbindgen_throw correlates with the order of the globals I mentioned above.

@daxpedda daxpedda marked this pull request as draft October 7, 2024 08:53
@daxpedda
Copy link
Collaborator

daxpedda commented Oct 7, 2024

Thank you for looking into this, it has been terribly annoying so far.
My observation so far is that it is actually deterministic (in the CI), as long as nothing changes.
This is still annoying because every PR has to adjust the raw.js file.

@daxpedda daxpedda mentioned this pull request Oct 10, 2024
@RunDevelopment
Copy link
Contributor Author

Closed in favor of #4190

@RunDevelopment RunDevelopment deleted the deterministic-CI branch October 12, 2024 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants