Add unit fast paths to Calendar.ISO.iso_days_to_unit/2 - #15771
Merged
josevalim merged 1 commit intoAug 20, 2026
Conversation
Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Thomas Cioppettini <544875+tomciopp@users.noreply.github.com>
Member
|
I also optimized the common path where the units are the same upstream: erlang/otp#11519 |
Member
|
💚 💙 💜 💛 ❤️ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Assisted-by: Claude Code:claude-fable-5
Continuing with the performance work, here is another case where we can short circuit.
System.convert_time_unit/3delegates to:erlang.convert_time_unit/3, which is plain Erlang code, and has no same unit short circuit. It always executes:So we are:
Calendar.ISO.iso_days_to_unit/2funnels every extraction throughSystem.convert_time_unit(total, :microsecond, unit). That call(
:erlang.convert_time_unit) has no same-unit short circuit: it computes(total * to_ppd) div from_ppd, so the identity:microsecondconversionhard-coded inside
NaiveDateTime.diff/3andDateTime.diff/3multiplies a~6.4e16 microsecond total by 1_000_000 — a ~6.4e22 bignum — and then divides
it right back. That happens twice per diff (three times for
:day/:hour/:minute, which route through:microsecond), andDateTime.to_unix/2paysthe same shape for
:millisecond.This adds dedicated
:second,:millisecond, and:microsecondclauses,mirroring the sibling
add_time_unit_to_iso_days/3directly below it(#15759). The
System.convert_time_unitfallback is kept for:native,:nanosecond, and integer units.The sub-second clauses use the module's existing
floor_div_positive_divisor/2rather than
div/2::erlang.convert_time_unitrounds toward negativeinfinity while
div/2truncates, andDateTime.to_unix/2produces negativetotals for every pre-1970 datetime. Since
to_unix/2had no test coveragewith non-default units, this also adds a test pinning that boundary
(
~U[1969-12-31 23:59:59.999999Z]is-1in:millisecond; truncatingdivision would return
0).Benchmark
Apple M5, macOS, OTP 29, JIT enabled. Averages over 3 s per job
(sub-microsecond medians quantize at the timer tick).
iso_days_to_unit(iso_days, :microsecond)(direct)Time.to_seconds_after_midnight/1DateTime.to_unix/1(:second)DateTime.to_unix/1pre-epoch (negative)DateTime.to_unix/2(:millisecond)DateTime.diff/2(:second, UTC)NaiveDateTime.diff/3(:microsecond)NaiveDateTime.diff/2(:second)NaiveDateTime.diff/3(:day)iso_days_to_unit(iso_days, :native)(fallback control)DateTime.to_unix/2(:native, fallback control)Benchmark script