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 miri builder #218

Merged
merged 1 commit into from Feb 9, 2022
Merged

Add miri builder #218

merged 1 commit into from Feb 9, 2022

Conversation

erickt
Copy link
Contributor

@erickt erickt commented Feb 4, 2022

This adds a builder to run cargo miri in order to check for certain classes of undefined behavior.

@cuviper
Copy link
Member

cuviper commented Feb 7, 2022

Any idea about that failure? I'm running locally and it's okay so far, but miri is also up to 10GB memory after 13 minutes. Maybe CI went OOM? Anyway, as long as there's no real issue there, we should probably #[cfg(not(miri))] some things so we don't inflate the CI time so much.

@erickt
Copy link
Contributor Author

erickt commented Feb 8, 2022

I think we're tripping over rust-lang/miri#1367, where the stacked borrow check is superlinear. We probably need to reduce the cycles for miri. I'll switch some of these expensive tests to that.

One interesting thing to note though that for map::tests::insert_2 uses about 2.2GB, but swapping in a HashMap, it only uses 338MB.

@erickt
Copy link
Contributor Author

erickt commented Feb 8, 2022

I pushed up a new version that uses lower limits when run inside miri. Hopefully that'll use less memory and run faster.

This adds a builder to run `cargo miri` in order to check for certain
classes of undefined behavior.

One thing to note is that we've had to parameterize a number of tests to
use a smaller cycle count under miri. This is because miri's stacked
borrows checker is superlinear:

rust-lang/miri#1367

This can cause the miri builder to run out of memory. To avoid this, we
reduce the cycle counts for a few tests that are particularly expensive
when we're running inside miri.
@cuviper
Copy link
Member

cuviper commented Feb 9, 2022

Thanks! The new 4m miri CI time is totally reasonable.

@cuviper cuviper merged commit d6a9dd6 into indexmap-rs:master Feb 9, 2022
@erickt erickt deleted the miri branch February 9, 2022 18:44
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.

None yet

2 participants