perf: skip initial null items and don't recompute slope in interpolate
#15819
Conversation
github-actions
bot
added
performance
MarcoGorelli
force-pushed
the
interpolate-skip-first
branch
from
44cfad9
to
af4dec6
Compare
CodSpeed Performance ReportMerging #15819 will not alter performanceComparing Summary
|
MarcoGorelli
requested review from
ritchie46,
stinodego,
orlp and
c-peters
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #15819 +/- ##
==========================================
- Coverage 80.38% 80.37% -0.02%
==========================================
Files 1263 1263
Lines 165346 165346
==========================================
- Hits 132918 132895 -23
- Misses 32428 32451 +23 ☔ View full report in Codecov by Sentry. |
| let diff = high - low; | ||
| let slope = (high - low) / steps_n; | ||
| for step_i in 1..steps { | ||
| let step_i: T = NumCast::from(step_i).unwrap(); | ||
| let v = linear_itp(low, step_i, diff, steps_n); | ||
| let v = linear_itp(low, step_i, slope); |
There was a problem hiding this comment.
diff / steps_n is independent of the loop, it doesn't need recomputing each time
MarcoGorelli
changed the title
slope in interpolate
| @@ -77,7 +77,7 @@ where | |||
|
|
|||
| // Fill av with first. | |||
| let mut av = Vec::with_capacity(chunked_arr.len()); | |||
| let mut iter = chunked_arr.into_iter(); | |||
| let mut iter = chunked_arr.into_iter().skip(first); | |||
There was a problem hiding this comment.
Can we use iter here. That should be faster.
|
This one is good to go. Thank yo @MarcoGorelli. There is more performance to be gained here for a future PR, if someone is up for it. Currently we iterator with |
The first
firstitems have already been checked to be null, there's no need to match on them twiceAlso, the slope was being repeatedly recomputed for each element of the for-loop