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

Memory use grows over time #3080

Closed
5 tasks done
saethlin opened this issue Sep 24, 2023 · 3 comments
Closed
5 tasks done

Memory use grows over time #3080

saethlin opened this issue Sep 24, 2023 · 3 comments
Assignees

Comments

@saethlin
Copy link
Member

saethlin commented Sep 24, 2023

There are a few places in both the interpreter and the additional Miri runtime where we accumulate state and never release it.

For example, this program uses more memory over time:

fn main() {
    for _ in 0..1_000_000 {}
}

(I like this program because it seems comically simple but of course ranges are quite complicated without any optimization)

There are a few sources of this memory growth. I'm going to try to fix or reduce as many as possible. In most cases the growth is manageable but I have a real world case someone showed me where we hit ~10 GB in just a few minutes due to these issues.

  • Stacked Borrows Allocation history and in particular the invalidations is the biggest offender. I think we can fix this with the tag GC. I'll send a PR tomorrow.
  • The spans we save for all allocations/locals are probably next. Not really sure what to do here, maybe also use the tag GC + the set of exposed allocs.
  • Intptrcast state cannot just be cleaned up on deallocation. I tried doing that and I didn't get the test failures I was hoping to see. Perhaps the tag GC can help us again here, or maybe we can only clean up one side of the mapping.
  • The dead allocs map in the interpreter. Based on the comments, I don't think we need this in Miri?
  • Return a finite number of AllocIds per ConstAllocation in Miri rust#118336

The trickiest thing about these is that so far it looks like fixing these problems does not make Miri any faster or slower, and we don't have peak memory benchmarks. And hyperfine doesn't support them.

@saethlin saethlin self-assigned this Sep 24, 2023
@RalfJung
Copy link
Member

Intptrcast state cannot just be cleaned up on deallocation. I tried doing that and I didn't get the test failures I was hoping to see. Perhaps the tag GC can help us again here, or maybe we can only clean up one side of the mapping.

This would need an AllocId GC instead of tag GC, and even then I am not sure whether it is sufficient. We'd have to be very sure that future int2ptr calls behave the same before after after cleanup.

The dead allocs map in the interpreter. Based on the comments, I don't think we need this in Miri?

It will be used at least with -Zmiri-symbolic-alignment.

bors added a commit that referenced this issue Sep 25, 2023
GC the Stacked Borrows allocation history

This handles the biggest contributor to #3080

The benchmark that this adds demonstrates the memory improvement here, but our benchmark setup doesn't record memory usage, and `hyperfine` doesn't support emitting memory usage stats. I ran this benchmark manually with `/usr/bin/time -v cargo +miri miri run` 🤷
RalfJung pushed a commit to RalfJung/rust that referenced this issue Sep 28, 2023
GC the Stacked Borrows allocation history

This handles the biggest contributor to rust-lang/miri#3080

The benchmark that this adds demonstrates the memory improvement here, but our benchmark setup doesn't record memory usage, and `hyperfine` doesn't support emitting memory usage stats. I ran this benchmark manually with `/usr/bin/time -v cargo +miri miri run` 🤷
@saethlin
Copy link
Member Author

saethlin commented Oct 3, 2023

We'd have to be very sure that future int2ptr calls behave the same before after after cleanup.

This is what I was getting at with the "+ the set of exposed allocs". I think expose_addr is simply asking for a leak, and so it shall be in the interpreter.

bors added a commit to rust-lang-ci/rust that referenced this issue Nov 20, 2023
Expand Miri's BorTag GC to a Provenance GC

As suggested in rust-lang/miri#3080 (comment)

We previously solved memory growth issues associated with the Stacked Borrows and Tree Borrows runtimes with a GC. But of course we also have state accumulation associated with whole allocations elsewhere in the interpreter, and this PR starts tackling those.

To do this, we expand the visitor for the GC so that it can visit a BorTag or an AllocId. Instead of collecting all live AllocIds into a single HashSet, we just collect from the Machine itself then go through an accessor `InterpCx::is_alloc_live` which checks a number of allocation data structures in the core interpreter. This avoids the overhead of all the inserts that collecting their keys would require.

r? `@RalfJung`
compiler-errors added a commit to compiler-errors/rust that referenced this issue Nov 21, 2023
Expand Miri's BorTag GC to a Provenance GC

As suggested in rust-lang/miri#3080 (comment)

We previously solved memory growth issues associated with the Stacked Borrows and Tree Borrows runtimes with a GC. But of course we also have state accumulation associated with whole allocations elsewhere in the interpreter, and this PR starts tackling those.

To do this, we expand the visitor for the GC so that it can visit a BorTag or an AllocId. Instead of collecting all live AllocIds into a single HashSet, we just collect from the Machine itself then go through an accessor `InterpCx::is_alloc_live` which checks a number of allocation data structures in the core interpreter. This avoids the overhead of all the inserts that collecting their keys would require.

r? `@RalfJung`
Noratrieb added a commit to Noratrieb/rust that referenced this issue Nov 21, 2023
Expand Miri's BorTag GC to a Provenance GC

As suggested in rust-lang/miri#3080 (comment)

We previously solved memory growth issues associated with the Stacked Borrows and Tree Borrows runtimes with a GC. But of course we also have state accumulation associated with whole allocations elsewhere in the interpreter, and this PR starts tackling those.

To do this, we expand the visitor for the GC so that it can visit a BorTag or an AllocId. Instead of collecting all live AllocIds into a single HashSet, we just collect from the Machine itself then go through an accessor `InterpCx::is_alloc_live` which checks a number of allocation data structures in the core interpreter. This avoids the overhead of all the inserts that collecting their keys would require.

r? ``@RalfJung``
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Nov 21, 2023
Rollup merge of rust-lang#118029 - saethlin:allocid-gc, r=RalfJung

Expand Miri's BorTag GC to a Provenance GC

As suggested in rust-lang/miri#3080 (comment)

We previously solved memory growth issues associated with the Stacked Borrows and Tree Borrows runtimes with a GC. But of course we also have state accumulation associated with whole allocations elsewhere in the interpreter, and this PR starts tackling those.

To do this, we expand the visitor for the GC so that it can visit a BorTag or an AllocId. Instead of collecting all live AllocIds into a single HashSet, we just collect from the Machine itself then go through an accessor `InterpCx::is_alloc_live` which checks a number of allocation data structures in the core interpreter. This avoids the overhead of all the inserts that collecting their keys would require.

r? ``@RalfJung``
bors pushed a commit that referenced this issue Nov 21, 2023
Expand Miri's BorTag GC to a Provenance GC

As suggested in #3080 (comment)

We previously solved memory growth issues associated with the Stacked Borrows and Tree Borrows runtimes with a GC. But of course we also have state accumulation associated with whole allocations elsewhere in the interpreter, and this PR starts tackling those.

To do this, we expand the visitor for the GC so that it can visit a BorTag or an AllocId. Instead of collecting all live AllocIds into a single HashSet, we just collect from the Machine itself then go through an accessor `InterpCx::is_alloc_live` which checks a number of allocation data structures in the core interpreter. This avoids the overhead of all the inserts that collecting their keys would require.

r? ``@RalfJung``
@saethlin
Copy link
Member Author

saethlin commented Feb 1, 2024

Since rust-lang/rust#118336 landed, I think this is done. I don't observe memory growth anymore.

@saethlin saethlin closed this as completed Feb 1, 2024
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

No branches or pull requests

2 participants