Shrink QueryRevisions size by 3 words
#636
Conversation
✅ Deploy Preview for salsa-rs canceled.
|
CodSpeed Performance ReportMerging #636 will not alter performanceComparing Summary
|
|
I wonder if #629 would help here a bit by reducing the need to allocate additionally (or whether the cause of the perf regression is the extra indirection) |
|
I haven't reviewed the code changes but any chance you undid the optimization introduced in #615 |
|
Ah I believe I did by accident! |
Veykril
force-pushed
the
veykril/push-accumulated
branch
2 times, most recently
from
d0b5fd4 to
96d7780
Compare
Veykril
force-pushed
the
veykril/push-accumulated
branch
from
96d7780 to
009c91f
Compare
| ) -> Option<&'db AccumulatedMap>; | ||
| ) -> (Option<&'db AccumulatedMap>, InputAccumulatedValues) { | ||
| _ = (db, key_index); | ||
| (None, InputAccumulatedValues::Any) | ||
| } |
There was a problem hiding this comment.
One thing to note is that I have this trait method a default impl to reduce some code noise. I imagine the intent was to force the implementor to think about this function but it does make changing signatures really annoying as well.
Veykril
force-pushed
the
veykril/push-accumulated
branch
from
009c91f to
0537864
Compare
Veykril
force-pushed
the
veykril/push-accumulated
branch
from
0537864 to
dfec276
Compare
| @@ -7,10 +9,6 @@ use super::{accumulated::Accumulated, Accumulator, AnyAccumulated}; | |||
| #[derive(Default, Debug)] | |||
| pub struct AccumulatedMap { | |||
| map: FxHashMap<IngredientIndex, Box<dyn AnyAccumulated>>, | |||
There was a problem hiding this comment.
I guess a more local change would be to change map here to Option<Box<FxHashMap>>. It would have the advantage that all accumualted fields remain closer together.
There was a problem hiding this comment.
Generally not a bad idea but I think (will have to verify but I am fairly certain) that moving InputAccumulatedValues back into AccumulatedMap will increase the size of QueryRevisions as the field can't be batched together with other fields that are smaller than the alignment.
|
This was a pretty substantial improvement to rust-analyzer's memory benchmark ( |
|
We could save another 24 bytes by option boxing |
|
And another 8 bytes by having access to a |
Veykril
force-pushed
the
veykril/push-accumulated
branch
from
dfec276 to
4abb000
Compare
Veykril
force-pushed
the
veykril/push-accumulated
branch
from
4abb000 to
eb4340b
Compare
No description provided.