Repository navigation
Large memory regression #8589
Description
It appears that #8235 caused a ~500MB memory regression.
The plot is very noisy because there were a few non-bors pushes to master, and the graph is purely time-based, so commits are shown when they are authored, not merged; but efd6eaf (the last "low" commit) is the parent of 2a7be1c (the merge of #8235) so I'm fairly sure it was caused by that PR, quite possibly the visitor changes.
Activity
nejucomo commentedon Sep 29, 2013
It looks like the memory profiling on Is Rust Slim Yet? profiles running
rustconlibrustc. This does not distinguish whether the memory leak is in the rustc application code, the libraries it depends on (such asstd), the runtime support, or in generated code.Can anyone suggest a way to distinguish these cases? For example, if there is similar memory profiling for
servo, which is another probably complex application, that would suggeststd,extra, runtime support, or generated code, but notrustcapplication code itself. If there is not a correlated memory leak inservo, then maybe this suggests the leak is inrustcitself.That's not very precise, but if the data is already there it could suggest where to look. How can I learn about memory profiling tools that work well on rust code?
Anyway, I'm going to read over the diff between the parents of 2a7be1c which are efd6eaf and 9457ebe to see if I notice anything suspicious by code inspection.
alexcrichton commentedon Oct 2, 2013
I believe that I've found the regression. It's because one of the lint passes is holding onto a lint Context, and the lint Context is holding onto a pass, resulting in a cycle. After this, the tcx held by the Context is never free'd.
@cmr graciously created a memory graph where no lint passes are run. I'm working on a fix.
emberian commentedon Oct 2, 2013
@alexcrichton so it's not ideal, but could the lint passes hold a
&'staticto the context? It'll always outlive them, and it's be a gig of memory savings.alexcrichton commentedon Oct 2, 2013
Taking a look at lint, lint has a large number of problems with it otherwise, so I'd rather just do this right this time around. I should have a pull up by tonight.
Rewrite lint passes with less visitor cruft
auto merge of #9686 : alexcrichton/rust/lint, r=thestinger