perf: Make dt.truncate 1.5x faster when every is just a single duration (and not an expression)

[MarcoGorelli]

broadcast_try_binary_elementwise simplifies things here, but does seem to introduce a perf hit for the base case (when every isn't an expression): #15768 (comment)

this makes a noticeable difference for the single-every case https://www.kaggle.com/code/marcogorelli/polars-timing?scriptVersionId=181122731:

• here: 0.05906398600003134
• main: 0.09365042766664071

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.49%. Comparing base (ef64730) to head (61f5d18).
Report is 9 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #16666      +/-   ##
==========================================
- Coverage   81.51%   81.49%   -0.02%     
==========================================
  Files        1414     1415       +1     
  Lines      186392   186609     +217     
  Branches     3014     3014              
==========================================
+ Hits       151942   152083     +141     
- Misses      33921    33996      +75     
- Partials      529      530       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[MarcoGorelli]

i've made an issue for this #16617

[ritchie46]

What does the seperate implementation do that we cannot do in the broadcast_ function? Could we improve that helper function?

[MarcoGorelli]

In the broadcast one, the function:

• takes a single lhs_opt and a single rhs_opt
• performs some logic with them

If rhs_opt is the same each time, then some of that logic may get duplicated. Here, for example, this part would get repeated each time:

                let every =
                    *duration_cache.get_or_insert_with(every, |every| Duration::parse(every));
                if every.negative {
                    polars_bail!(ComputeError: "cannot truncate a Datetime to a negative duration")
                }

The cache would avoid the cost of Duration::parse on each element, but it still involves looking up every in the cache each time, and checking if every is negative. Doing this each time adds up

If every.len() == 1, then every just needs parsing as a duration once, and then it's just a matter of running w.truncate_*s on each element

I think that the broadcast_ probably can be improved, but that ultimately just keeping it simple will always be fastest?

[ritchie46]

but that ultimately just keeping it simple will always be fastest?

Yes.. Thanks for the explanation. 👍