Slow the rate of growth in ConsolidatingVec
#31077
Conversation
|
From Slack, @teskje recommends a launch darkly flag to allow the number to be configurable, in case two was very important for some reasons (plausible!). I'll need to chase down what threading that through looks like. |
| /// Denominator in the growth rate, where 2 corresponds to doubling, and `n` to `1 + 1/(n-1)`. | ||
| /// | ||
| /// If consolidation didn't free enough space, at least a linear amount, increase the capacity | ||
| /// The `slop` term describes the rate of growth, where we scale up by factors of 1/slop. |
There was a problem hiding this comment.
| /// The `slop` term describes the rate of growth, where we scale up by factors of 1/slop. | |
| /// The `slop` term describes the rate of growth, where we scale up by factors of 1/(slop-1). |
... right?
| /// The denominator `n` in the growth rate `1 + 1/(n-1)` for `ConsolidatingVec` growth. | ||
| pub const CONSOLIDATING_VEC_GROWTH_DENOMINATOR: Config<usize> = Config::new( | ||
| "consolidating_vec_growth_denominator", | ||
| 2, |
There was a problem hiding this comment.
I'd be okay with defaulting this to our target (11?) and then opting out were we observe issues. Seems safe enough, considering that this trades of less memory for more CPU and we usually don't have CPU issues.
| /// Configuration for `ConsolidatingVec` determining the rate of growth (doubling, or less). | ||
| growth_denominator: usize, |
There was a problem hiding this comment.
There isn't anything that enforces "doubling or less" here, right? People could set the config to 1 or 0 and end up having a bad time. How about checking that in new and falling back to 2 on invalid values? Alternatively, redefine the semantics to have 0 mean doubling.
The growth of
ConsolidatingVecwas by doubling, which meant that we can end up with substantially more memory use than we really need. This PR attempts to make this configurable, growing by a smaller factor determined by a positive integerslop. If the result of consolidation is (1 - 1/slop)-full, we grow the vector by a factor of 1 + 1/(slop-1). These relationships were chosen to match the 2x behavior, but it's possible that they are imperfect.I'm not certain how to test the impact of this, and whether we have "MV rehydration memory" tests, but that would be the right way to see if this is helpful. It comes at the potential expense of CPU.
Motivation
Tips for reviewer
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.