core: Potential optimization of ModuleExpansionTransformer#31302
core: Potential optimization of ModuleExpansionTransformer#31302apparentlymart wants to merge 3 commits intomainfrom
Conversation
This aims to amortize the cost of unique-keying and comparing module addresses by doing a single up-front transform of all of them into UniqueKey values. Internally these UniqueKey values are really just string addresses and so comparing these boils down to a flat string comparison. I only wrote this up to see if it would be possible to do it, and so I've not actually measured if it's any faster this way. I'll need to develop a realistic benchmark to measure this with first before we can decide if this is a productive optimization.
We originally wrote these to take a *testing.T directly, but that means we can't use these utilities in benchmarks or fuzz tests because those use their own separate *testing.B and *testing.F controller types. The testing package offers an interface testing.TB that includes the methods that all three of these types have in common, and our test utilities for configuration loading only need a subset of those methods so using that interface instead of the concrete types will allow us to share our helper functions across all three situations.
apparentlymart
force-pushed
the
f-module-expansion-optimize
branch
from
6cc561a to
b40d0fd
Compare
|
I had a little more spare time at the end of the day today, and so I tried for a small benchmark to see if I could show this making a significant difference. My benchmark is at a different scale of modules and module nesting than #30968 just because I was trying to do it with a traditional test fixture rather than some other strategy like a synthetically-generated, large and exponentially nested configuration. Therefore it's worth taking with a pinch of salt, but the summary of my findings is that this change did make a marginal difference compared to the commit I originally wrote it against, but by coincidence in the meantime we merged #31293 which optimizes the underlying Of course the code here will live on in case we want to try other optimizations against this codepath in future, or in case we decide to write a more realistic "stress test" benchmark that might help the amortization of this PR show better. |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
1 participant
This aims to amortize the cost of unique-keying and comparing module addresses by doing a single up-front transform of all of them into
UniqueKeyvalues. This was motivated by noticing some potential low-hanging fruit in the report over in #30968.Internally these
UniqueKeyvalues are really just string addresses and so comparing these boils down to just a string comparison in practice.I only wrote this up to see if it would be possible to do it, and so I've not actually measured if it's any faster this way. I'll need to develop a realistic benchmark to measure this with first before we can decide if this is a productive optimization. This didn't fit so neatly into my remaining work day as I'd hoped and so I've run out of time for now but might poke at this some more later.