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

Add sorted_iter_mut for NonstandardWitSection.adapters for determinism #3851

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

Aaron1011
Copy link
Contributor

@Aaron1011 Aaron1011 commented Feb 22, 2024

In the externref code, we call export_xform while iterating over adapters with iter_mut. Calling export_xform can result in shims, with the id depending on the order in which we iterate over the exports.

This makes the .wasm output deterministic when using --reference-types

Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we instead introduce a sorted_iter_mut()?

I've never looked at this part of the code, but the way ordered iteration is done is kinda awful. Apart from a proper fix to all these cases I would prefer to stick with the current implementation.

@Aaron1011 Aaron1011 changed the title Use BTreeMap for NonstandardWitSection.adapters for determinism Add sorted_iter_mut for NonstandardWitSection.adapters for determinism Feb 22, 2024
@Aaron1011
Copy link
Contributor Author

I've updated this to add a new sorted_iter_mut function

Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies, forgot to mention the changelog, could you add an entry there considering that this change might actually be relevant for users?

Otherwise LGTM.

In the `externref` code, we call `export_xform` while iterating
over `adapters` with `iter_mut`. Calling `export_xform` can result
in shims, with the id depending on the order in which we iterate
over the exports.

To make the final .wasm output deterministic, I've changed this to
use a new `sorted_iter_mut` method (based on `sorted_iter`).
@Aaron1011
Copy link
Contributor Author

I updated the changelog

Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@daxpedda daxpedda merged commit b5a74c8 into rustwasm:main Feb 22, 2024
24 of 25 checks passed
@torokati44
Copy link

Is there a plan, or estimation, on when 0.2.92 will come out, with this in it?
It would be great to have, to properly resolve ruffle-rs/ruffle#15294. 🙏

@daxpedda
Copy link
Collaborator

I will try to make a new release at the end of the week.

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.

3 participants