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

Revise VecPerParamSpace to use a one Vec rather than three #15418

Merged
merged 2 commits into from
Jul 5, 2014

Conversation

pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Jul 4, 2014

In my informal measurements, this brings the peak memory usage when
building librustc from 1662M down to 1502M. Since 1662 - 1502 = 160,
this may not recover the entirety of the observed memory regression
(250M) from PR #14604. (However, according to my local measurements,
the regression when building librustc was more like 209M, so perhaps
this will still recover the lions share of the lost memory.)

@pnkfelix
Copy link
Member Author

pnkfelix commented Jul 4, 2014

@pcwalton I think you were right, that the memory usage increase was due to malloc slop. (That, or my shrink_to_fit calls simply were not added as aggressively as they needed to be.)

@sfackler
Copy link
Member

sfackler commented Jul 4, 2014

There are some build errors in rustdoc: https://travis-ci.org/rust-lang/rust/jobs/29158017

//
// AF(self) = (self.content.slice_to(self.type_limit),
// self.content.slice(self.type_limit, self.self_limit),
// self.content.slice(self.self_limit, self.content.len()))
Copy link
Member

Choose a reason for hiding this comment

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

slice_from fwiw.

@pcwalton
Copy link
Contributor

pcwalton commented Jul 4, 2014

r=me with @huonw's comment change.

pnkfelix added 2 commits July 5, 2014 06:29
This basically meant changing the interface so that no borrowed `&Vec`
is exposed, by hiding `fn get_vec` and `fn get_mut_vec` and revising
`fn all_vecs`.

Instead, clients should use one of the other methods; `get_slice`,
`pop`, `truncate`, `replace`, `push_all`, or `is_empty_in`, which
should work for any case currently used in rustc.
In my informal measurements, this brings the peak memory usage when
building librustc from 1662M down to 1502M.  Since 1662 - 1502 = 160,
this may not recover the entirety of the observed memory regression
(250M) from PR rust-lang#14604.  (However, according to my local measurements,
the regression when building librustc was more like 209M, so perhaps
this will still recover the lions share of the lost memory.)
bors added a commit that referenced this pull request Jul 5, 2014
…cwalton

In my informal measurements, this brings the peak memory usage when
building librustc from 1662M down to 1502M.  Since 1662 - 1502 = 160,
this may not recover the entirety of the observed memory regression
(250M) from PR #14604.  (However, according to my local measurements,
the regression when building librustc was more like 209M, so perhaps
this will still recover the lions share of the lost memory.)
@bors bors closed this Jul 5, 2014
@bors bors merged commit 4459fe3 into rust-lang:master Jul 5, 2014
@huonw
Copy link
Member

huonw commented Jul 6, 2014

This did fix the vast majority of the regression: http://huonw.github.io/isrustfastyet/mem/#b00f4ec,e0d3cf6! (Saved 225 MB there, but there is some noise.)

@nikomatsakis
Copy link
Contributor

Interesting. This provides some evidence for a hypothesis of mine: that aggressive use of Box<[T]> to reflect vectors we were no longer planning to grow and Vec<T> for those we are may help memory usage overall. I've heard similar comments about the need to shrink to fit in Java apps to avoid memory bloat.

@pnkfelix
Copy link
Member Author

pnkfelix commented Jul 7, 2014

@nikomatsakis if that hypothesis were true, then it would have sufficed to add shrink_to_fit invocations at all the correct locations, to force the Vec's down to the proper size, correct? Or are you foreseeing some other benefit of Box<[T]> that is not captured by aggressive use of the shrink_to_fit method?

I mention this because adding shrink_to_fit calls (though admittedly not as aggressively as I could have), was the first thing I tried, and that did not seem to help very much. this is what led to @pcwalton and I discussing whether the cause was due to "malloc slop" (i.e. overhead from simply having several distinct malloc'ed blocks of memory and the fragmentation and/or metadata associated with those blocks, versus folding the distinct blocks together into one contiguous chunk.)

@nikomatsakis
Copy link
Contributor

I mention this because adding shrink_to_fit calls (though
admittedly not as aggressively as I could have), was the first
thing I tried, and that did not seem to help very much. this is
what led to @pcwalton and I
discussing
whether the cause was due to "malloc slop" (i.e. overhead from
simply having several distinct malloc'ed blocks of memory and the
fragmentation and/or metadata associated with those blocks, versus
folding the distinct blocks together into one contiguous chunk.)

I see, I didn't really read the patch and misunderstood what it did. Interesting!

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 21, 2023
Fix signature help of methods from macros

Currently the receiver type is copied from AST instead re-formatting through `HirDisplay`. Macro generated functions seem to have no spaces and their signature help are rendered like `fn foo(&'amutself)` instead of `fn foo(&'a mut self)`.
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.

6 participants