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

Miri's memory usage has decreased. Thoughts on giving the default profile some parallelism? #1346

Closed
saethlin opened this issue Mar 1, 2024 · 3 comments · Fixed by #1347
Closed

Comments

@saethlin
Copy link
Contributor

saethlin commented Mar 1, 2024

[profile.default-miri]
# Miri tests take up a lot of memory, so only run 1 test at a time by default.
test-threads = 1

Previously, I'd have just agreed with the statement in the default config. But I've landed a number of PRs over the past months, culminating with rust-lang/rust#118336 which have all reduced the interpreter's memory growth over time. The interpreter is still not lightweight by any means; unit tests seem to consume no less than 250 MB and I rarely see them using more than 500 MB.

But I don't think suppressing parallelism is a clearly superior default anymore. I'd bet that most development environments can support available_parallelism() Miri jobs now, but I expect you want to be more cautious than that. Thoughts?

@sunshowers
Copy link
Member

Sure, sounds good to me. I don't remember actively making this decision -- I think I went along with something either @RalfJung or you said.

Do you want to be more cautious and increase the parallelism to (say) 4, or do you just want to remove the constraint entirely and let the settings from the default profile take over?

At some point I'd love to add arithmetic operations, so you can say something like min(system_memory() / 2GiB, num_cpus()), but that's a far-future thing.

@saethlin
Copy link
Contributor Author

saethlin commented Mar 2, 2024

Yeah that arithmetic would be ideal. If the only options are full parallelism or a fixed number, I'd prefer full parallelism. I suspect it will Just Work for nearly everyone, and I have little grasp of how many people are missing out on nextest parallelism because of this default, but I suspect that's the more common experience.

@sunshowers
Copy link
Member

Sure. Would you like to submit a PR for this? I'm planning to release a new version soon.

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 a pull request may close this issue.

2 participants