-
Notifications
You must be signed in to change notification settings - Fork 5
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
SegmentCache #20
base: mccolljr/const_generics
Are you sure you want to change the base?
SegmentCache #20
Conversation
* make SegVec const-generic * SegVec::new can't be const because smallvec's new is non-const * Fix a bug where the truncate method improperly accounted for factor * Make some simple functions inline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks quite nice so far. Just a couple thoughts based on a first pass
#[cfg(test)] | ||
fn checked_log2_floor(v: usize) -> Option<u32> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is no longer used by the implementation, it is okay to remove it and the associated test.
/// Called by ctors to assert that the configuration is valid. | ||
/// | ||
/// Some `MemConfig` implementations may put constraints on (const generic) | ||
/// parameters. Currently it is impossible to assert these at compile time in stable | ||
/// rust. In debug builds we check these constraints when a `SegVec` uses some | ||
/// config. The three shipped `MemConfig` implementations check here that the 'FACTOR' is | ||
/// not zero. | ||
fn debug_assert_config(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are some stable ways to assert things about const generics now.
This is one example (stolen from a thread in an issue on static_assertions):
If you feel up to it, I think it would be better to have these assertions happen at compile time... especially given that the (current) assertions are simple, just checking the FACTOR value against 0.
On 2023-06-10 12:31, Jacob Ryan McCollum wrote:
@mccolljr commented on this pull request.
Looks quite nice so far. Just a couple thoughts based on a first pass
> +#[cfg(test)]
fn checked_log2_floor(v: usize) -> Option<u32> {
If this is no longer used by the implementation, it is okay to remove
it and the associated test.
Things are somewhat in flux still. I was thinking about keeping code
for now and eventually do some 'cleanup'. Would be nice to use
cargo-mutants to improve testing in future, but that's not my goal right
now.
> + /// Called by ctors to assert that the configuration is valid.
+ ///
+ /// Some `MemConfig` implementations may put constraints on
(const generic)
+ /// parameters. Currently it is impossible to assert these at
compile time in stable
+ /// rust. In debug builds we check these constraints when a
`SegVec` uses some
+ /// config. The three shipped `MemConfig` implementations check
here that the 'FACTOR' is
+ /// not zero.
+ fn debug_assert_config();
I think there are some stable ways to assert things about const
generics now.
This is one example (stolen from a thread in an issue on
static_assertions):
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=98221c637c7195f1fc530cc51d3848f8
If you feel up to it, I think it would be better to have these
assertions happen at compile time... especially given that the
(current) assertions are simple, just checking the FACTOR value
against 0.
I didn't know that such works. lets see about adding that later. Note
that I left the road open for user implementations of MemConfig. Dunno
if everything can be asserted. The debug-only test at runtime as it is
currently should be good enough anyway.
My priority is to get it usable (with the cache thing if possible)
and have performance in the same Ballpark than std Vec (somewhat slower
is to be expected of course).
Plan is to improve and help maintaining my bits in future, but for now
I just want to get it going and work on something else (the thing where
I need SegVec).
…
--
Reply to this email directly or view it on GitHub:
#20 (review)
You are receiving this because you authored the thread.
Message ID: ***@***.***>
|
* add mem_config.rs for different allocation policies * comment wording * The MemConfig impls must be public * add 'debug_assert_config' for validating config parameters * integrate MemConfig into SegVec This removes the 'capacity' member for now, it might be added back in future or be part of the planned 'cache' feature. Benchmarking will show. * fixing few clippy lints (some remain, to be addressed later) * split 'reserve()' into a hot and a cold path and add capacity back This gives a *significant* performance improvement. * before * push 10k values with default growth factor * time: [133.64 µs 133.96 µs 134.29 µs] * push 10k values with large growth factor * time: [81.332 µs 81.857 µs 82.335 µs] * after * push 10k values with default growth factor * time: [79.874 µs 80.206 µs 80.643 µs] * push 10k values with large growth factor * time: [46.552 µs 46.704 µs 46.826 µs] * improve benchmarks * Increase the loop to 1Mio (from 10k) * Add benchmars for * std Vec to have something to compare to * Linear/Proportional/Exponential with FACTOR 32 to compare between them
makes debugging/testing easier
not integrated yet
* Improves 'reserve()' again while at it. * only 'push_cached()' for testing * add cached benchmarks We are getting closer to std Vec speed!
I updated the benchmarks (not committed yet) and want to share some observations:
ConclusionProportional is too slow, needs better math, maybe something that is only 'roughly' proportional (I have no actual idea about that). While the caching idea works, it's gains are not as big as I hoped for and it is always outperformed by Linear<1024>. I will scrap the SegmentCache.
|
This might closed now, the code exists as proof-of-concept. It could be reasonable to revive this idea when (cacheable) fast indexing on Proportional or Exponential is required. Still chosing Linear would likely be the better approach. |
WIP! caching segments should improve performance considerably. integration and benching follows.