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

Shrink overallocated capacity if a number gets a lot smaller. #171

Merged
merged 1 commit into from
Oct 30, 2020
Merged

Shrink overallocated capacity if a number gets a lot smaller. #171

merged 1 commit into from
Oct 30, 2020

Conversation

tczajka
Copy link
Contributor

@tczajka tczajka commented Oct 24, 2020

If a number gets below 1/4 of capacity, shrink_to_fit. Since Vec grows by 2, this amortizes well
for any sequence of sizes.

If a number gets below 1/4 of capacity, shrink_to_fit. Since Vec grows by 2, this amortizes well
for any sequence of sizes.
@cuviper
Copy link
Member

cuviper commented Oct 30, 2020

Do you have any real-world data supporting this? I'm hoping to see both memory use and performance impact. The performance is probably fine, since it's a simple comparison of local fields, but it would be good to have measurements.

@tczajka
Copy link
Contributor Author

tczajka commented Oct 30, 2020

This is primarily motivated by my desire to have a theoretical upper bound worst case guarantee, rather than a gain on average.

If I am doing some huge calculation, and have n numbers of m bits, it is important for me to have an upper bound that says I'll be using O(nm) bits of allocated memory. Otherwise, using the library for such calculations would give me pause because I would never know if memory use won't blow up, have to trust a heuristic when it could be a guarantee.

Most benchmarks show no impact, some benchmarks show ~5% degradation (in CPU).

I suspect the impact is not because of the check, but because of the actual shrinking. Plus when we actually do shrink, there is no extra space left, and sometimes having extra space (even one digit) helps in later operations.

So using the shrink_to API (instead of shrink_to_fit) would probably help (but it's not stable API).

Similarly, when a number is first allocated, I speculate that adding a few words of extra capacity might help performance on benchmarks.

@cuviper
Copy link
Member

cuviper commented Oct 30, 2020

OK, I think that's reasonable, and we can keep an eye on using shrink_to and with_capacity for headroom later. I'm also not confident that all shrinking ops are covered by normalize, but there's no public promise here, so that seems ok for now.

bors r+

bors bot added a commit that referenced this pull request Oct 30, 2020
171: Shrink overallocated capacity if a number gets a lot smaller. r=cuviper a=tczajka

If a number gets below 1/4 of capacity, shrink_to_fit. Since Vec grows by 2, this amortizes well
for any sequence of sizes.

Co-authored-by: Tomek Czajka <tczajka@gmail.com>
@bors
Copy link
Contributor

bors bot commented Oct 30, 2020

Build failed:

@cuviper
Copy link
Member

cuviper commented Oct 30, 2020

bors retry

@bors
Copy link
Contributor

bors bot commented Oct 30, 2020

@bors bors bot merged commit 091fa96 into rust-num:master Oct 30, 2020
@tczajka tczajka deleted the shrink branch October 30, 2020 19:33
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.

2 participants