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

Investigate dynamic Vec growth strategy for jemalloc hugs #27627

Closed
Gankra opened this issue Aug 10, 2015 · 9 comments
Closed

Investigate dynamic Vec growth strategy for jemalloc hugs #27627

Gankra opened this issue Aug 10, 2015 · 9 comments
Labels
A-collections Area: `std::collection` C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Gankra
Copy link
Contributor

Gankra commented Aug 10, 2015

Facebook's alternative C++ STL uses a rather dynamic strategy to ostensibly be friendly for common workloads and jemalloc's size classes. While most of FBVector's stuff is hacking around the fact the move/copy constructors are Awful, this seems applicable to us.

See:

@Gankra
Copy link
Contributor Author

Gankra commented Aug 10, 2015

Note that our strategy current boils down to:

if cap == 0 { 4 } else { 2 * cap }

See: http://doc.rust-lang.org/nightly/src/alloc/raw_vec.rs.html#138-214

Doubling in conjuction with with mem::size_of::<T>() * cap < isize::MAX as an invariant makes size computation basically as efficient as possible.

The cap == 0 check needs to be made anyway to decide if to do an alloc or realloc.

For reference, the change to would be to instead do something like:

if cap == 0 { 
    let new_cap = cmp::max(64 / mem::size_of::<T>(),  1);
    alloc(new_cap);
} else { 
    let new_cap = if pretty_small_or_pretty_big::<T>(cap) { // see FBVector source for details
        cap * 2
    } else { 
        // No need to check match because we're not `pretty_big` (< 1 MB)
        (cap * 3 + 1) / 2 
    };
    realloc(cap, new_cap);
}

This should be a new method, as double is still useful for VecDeque (which requires power of two cap).

@nagisa
Copy link
Member

nagisa commented Aug 10, 2015

I remember hearing that grow factor of 1.5 is better than 2, but I don't remember where it was or whether it is confirmed in any way.

@jdm
Copy link
Contributor

jdm commented Aug 10, 2015

https://blog.mozilla.org/nnethercote/2014/11/04/please-grow-your-buffers-exponentially/ is useful reading. There's a comment thread about the 1.5 vs. doubling thing, too.

@Gankra
Copy link
Contributor Author

Gankra commented Aug 10, 2015

@nagisa this is the assertion of the second-last link in the OP -- however the last link found negligible gains for Rust in some benchmark.

May want to scour e.g. https://github.com/facebook/folly/blob/master/folly/test/FBVectorTestBenchmarks.cpp.h for interesting workloads.

@Gankra
Copy link
Contributor Author

Gankra commented Aug 13, 2015

Note that applying this logic to with_capacity could be a perf hazard when wanting to do things like convert to a Box.

@gnzlbg
Copy link
Contributor

gnzlbg commented Nov 2, 2015

The only interesting workloads in FBVectorTestBenchmarks.cpp.h#L337 seem to be the pushBack and insert benchmark since reserve should allocate the exact amount a user specifies.

@steveklabnik steveklabnik added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed A-libs labels Mar 24, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 22, 2017
@steveklabnik
Copy link
Member

Triage: it's been almost four years; doesn't seem like anyone has looked into this. We don't use jemalloc by default anymore.

@gnzlbg
Copy link
Contributor

gnzlbg commented Sep 17, 2019

cc @nnethercote

@jonas-schievink jonas-schievink added A-collections Area: `std::collection` I-slow Issue: Problems and improvements with respect to performance of generated code. labels Sep 17, 2019
@Mark-Simulacrum
Copy link
Member

Closing in favor of #29931 since it seems at least "mostly the same" and given the lack of ongoing effort two bugs seems not useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-collections Area: `std::collection` C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants