Skip to content

Commit

Permalink
Normative: Do away with CalculateOffsetShift in Duration.compare
Browse files Browse the repository at this point in the history 
CalculateOffsetShift is only used in Duration.compare, when relativeTo is
a ZonedDateTime. Calling it twice performs two ZonedDateTime additions,
which are potentially user-observable method calls. If we are performing
these two additions anyway, we may as well just add each duration to
relativeTo and compare the timestamps. This saves a lot of other
user-observable calls.

It also allows removing the offset-shift parameter from
TotalDurationNanoseconds, because now it is always zero. That parameter
was confusing anyway.
  • Loading branch information
@ptomato
ptomato committed Aug 22, 2023
1 parent 910d37a commit e34ef5b
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 84 deletions.
56 changes: 50 additions & 6 deletions polyfill/lib/duration.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ import {
MILLISECONDS,
MICROSECONDS,
NANOSECONDS,
CALENDAR,
INSTANT,
TIME_ZONE,
CreateSlots,
GetSlot,
SetSlot
Expand Down Expand Up @@ -621,15 +624,56 @@ export class Duration {
}
let relativeTo = ES.ToRelativeTemporalObject(options);

const shift1 = ES.CalculateOffsetShift(relativeTo, y1, mon1, w1, d1);
const shift2 = ES.CalculateOffsetShift(relativeTo, y2, mon2, w2, d2);
if (y1 !== 0 || y2 !== 0 || mon1 !== 0 || mon2 !== 0 || w1 !== 0 || w2 !== 0) {
if (ES.IsTemporalZonedDateTime(relativeTo)) relativeTo = ES.ToTemporalDate(relativeTo);
const calendarUnitsPresent = y1 !== 0 || y2 !== 0 || mon1 !== 0 || mon2 !== 0 || w1 !== 0 || w2 !== 0;

if (ES.IsTemporalZonedDateTime(relativeTo) && (calendarUnitsPresent || d1 != 0 || d2 !== 0)) {
const instant = GetSlot(relativeTo, INSTANT);
const timeZone = GetSlot(relativeTo, TIME_ZONE);
const calendar = GetSlot(relativeTo, CALENDAR);
const precalculatedDateTime = ES.GetPlainDateTimeFor(timeZone, instant, calendar);

const after1 = ES.AddZonedDateTime(
instant,
timeZone,
calendar,
y1,
mon1,
w1,
d1,
h1,
min1,
s1,
ms1,
µs1,
ns1,
precalculatedDateTime
);
const after2 = ES.AddZonedDateTime(
instant,
timeZone,
calendar,
y2,
mon2,
w2,
d2,
h2,
min2,
s2,
ms2,
µs2,
ns2,
precalculatedDateTime
);
return ES.ComparisonResult(after1.minus(after2).toJSNumber());
}

if (calendarUnitsPresent) {
// relativeTo is PlainDate or undefined
({ days: d1 } = ES.UnbalanceDateDurationRelative(y1, mon1, w1, d1, 'day', relativeTo));
({ days: d2 } = ES.UnbalanceDateDurationRelative(y2, mon2, w2, d2, 'day', relativeTo));
}
ns1 = ES.TotalDurationNanoseconds(d1, h1, min1, s1, ms1, µs1, ns1, shift1);
ns2 = ES.TotalDurationNanoseconds(d2, h2, min2, s2, ms2, µs2, ns2, shift2);
ns1 = ES.TotalDurationNanoseconds(d1, h1, min1, s1, ms1, µs1, ns1);
ns2 = ES.TotalDurationNanoseconds(d2, h2, min2, s2, ms2, µs2, ns2);
return ES.ComparisonResult(ns1.minus(ns2).toJSNumber());
}
}
Expand Down
44 changes: 5 additions & 39 deletions polyfill/lib/ecmascript.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2533,7 +2533,7 @@ export function TemporalDurationToString(
) {
const sign = DurationSign(years, months, weeks, days, hours, minutes, seconds, ms, µs, ns);

let total = TotalDurationNanoseconds(0, 0, 0, seconds, ms, µs, ns, 0);
let total = TotalDurationNanoseconds(0, 0, 0, seconds, ms, µs, ns);
({ quotient: total, remainder: ns } = total.divmod(1000));
({ quotient: total, remainder: µs } = total.divmod(1000));
({ quotient: seconds, remainder: ms } = total.divmod(1000));
Expand Down Expand Up @@ -3195,17 +3195,7 @@ export function BalanceTime(hour, minute, second, millisecond, microsecond, nano
};
}

export function TotalDurationNanoseconds(
days,
hours,
minutes,
seconds,
milliseconds,
microseconds,
nanoseconds,
offsetShift
) {
if (days !== 0) nanoseconds = bigInt(nanoseconds).subtract(offsetShift);
export function TotalDurationNanoseconds(days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds) {
hours = bigInt(hours).add(bigInt(days).multiply(24));
minutes = bigInt(minutes).add(hours.multiply(60));
seconds = bigInt(seconds).add(minutes.multiply(60));
Expand Down Expand Up @@ -3356,7 +3346,7 @@ export function BalancePossiblyInfiniteTimeDuration(
nanoseconds,
largestUnit
) {
nanoseconds = TotalDurationNanoseconds(days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds, 0);
nanoseconds = TotalDurationNanoseconds(days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds);

const sign = nanoseconds.lesser(0) ? -1 : 1;
nanoseconds = nanoseconds.abs();
Expand Down Expand Up @@ -3780,21 +3770,6 @@ export function BalanceDateDurationRelative(years, months, weeks, days, largestU
};
}

export function CalculateOffsetShift(relativeTo, y, mon, w, d) {
if (IsTemporalZonedDateTime(relativeTo)) {
const instant = GetSlot(relativeTo, INSTANT);
const timeZone = GetSlot(relativeTo, TIME_ZONE);
const calendar = GetSlot(relativeTo, CALENDAR);
const offsetBefore = GetOffsetNanosecondsFor(timeZone, instant);
const after = AddZonedDateTime(instant, timeZone, calendar, y, mon, w, d, 0, 0, 0, 0, 0, 0);
const TemporalInstant = GetIntrinsic('%Temporal.Instant%');
const instantAfter = new TemporalInstant(after);
const offsetAfter = GetOffsetNanosecondsFor(timeZone, instantAfter);
return offsetAfter - offsetBefore;
}
return 0;
}

export function CreateNegatedTemporalDuration(duration) {
const TemporalDuration = GetIntrinsic('%Temporal.Duration%');
return new TemporalDuration(
Expand Down Expand Up @@ -5317,16 +5292,7 @@ export function AdjustRoundedDurationDays(
// duration, there's no way for another full day to come from the next
// round of rounding. And if it were possible (e.g. contrived calendar
// with 30-minute-long "days") then it'd risk an infinite loop.
let timeRemainderNs = TotalDurationNanoseconds(
0,
hours,
minutes,
seconds,
milliseconds,
microseconds,
nanoseconds,
0
);
let timeRemainderNs = TotalDurationNanoseconds(0, hours, minutes, seconds, milliseconds, microseconds, nanoseconds);
const direction = MathSign(timeRemainderNs.toJSNumber());

const timeZone = GetSlot(zonedRelativeTo, TIME_ZONE);
Expand Down Expand Up @@ -5445,7 +5411,7 @@ export function RoundDuration(
// If rounding relative to a ZonedDateTime, then some days may not be 24h.
let dayLengthNs;
if (unit === 'year' || unit === 'month' || unit === 'week' || unit === 'day') {
nanoseconds = TotalDurationNanoseconds(0, hours, minutes, seconds, milliseconds, microseconds, nanoseconds, 0);
nanoseconds = TotalDurationNanoseconds(0, hours, minutes, seconds, milliseconds, microseconds, nanoseconds);
let deltaDays;
if (zonedRelativeTo) {
const intermediate = MoveRelativeZonedDateTime(zonedRelativeTo, years, months, weeks, days);
Expand Down
63 changes: 24 additions & 39 deletions spec/duration.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,19 +97,29 @@ <h1>Temporal.Duration.compare ( _one_, _two_ [ , _options_ ] )</h1>
1. If _one_.[[Years]] = _two_.[[Years]], and _one_.[[Months]] = _two_.[[Months]], and _one_.[[Weeks]] = _two_.[[Weeks]], and _one_.[[Days]] = _two_.[[Days]], and _one_.[[Hours]] = _two_.[[Hours]], and _one_.[[Minutes]] = _two_.[[Minutes]], and _one_.[[Seconds]] = _two_.[[Seconds]], and _one_.[[Millieconds]] = _two_.[[Millieconds]], and _one_.[[Microseconds]] = _two_.[[Microseconds]], and _one_.[[Nanoseconds]] = _two_.[[Nanoseconds]], then
1. Return *+0*<sub>𝔽</sub>.
1. Let _relativeTo_ be ? ToRelativeTemporalObject(_options_).
1. Let _shift1_ be ? CalculateOffsetShift(_relativeTo_, _one_.[[Years]], _one_.[[Months]], _one_.[[Weeks]], _one_.[[Days]]).
1. Let _shift2_ be ? CalculateOffsetShift(_relativeTo_, _two_.[[Years]], _two_.[[Months]], _two_.[[Weeks]], _two_.[[Days]]).
1. If _one_.[[Years]] &ne; 0, or _two_.[[Years]] &ne; 0, or _one_.[[Months]] &ne; 0, or _two_.[[Months]] &ne; 0, or _one_.[[Weeks]] &ne; 0, or _two_.[[Weeks]] &ne; 0, then
1. If _relativeTo_ is an Object with an [[InitializedTemporalZonedDateTime]] internal slot, set _relativeTo_ to ? ToTemporalDate(_relativeTo_).
1. Let _calendarUnitsPresent_ be *false*.
1. If _one_.[[Years]] &ne; 0, or _two_.[[Years]] &ne; 0, or _one_.[[Months]] &ne; 0, or _two_.[[Months]] &ne; 0, or _one_.[[Weeks]] &ne; 0, or _two_.[[Weeks]] &ne; 0, set _calendarUnitsPresent_ to *true*.
1. If _relativeTo_ is an Object with an [[InitializedTemporalZonedDateTime]] internal slot, and either _calendarUnitsPresent_ is *true*, or _one_.[[Days]] &ne; 0, or _two_.[[Days]] &ne; 0, then
1. Let _timeZone_ be _relativeTo_.[[TimeZone]].
1. Let _calendar_ be _relativeTo_.[[Calendar]].
1. Let _instant_ be ! CreateTemporalInstant(_relativeTo_.[[Nanoseconds]]).
1. Let _precalculatedDateTime_ be ? GetPlainDateTimeFor(_timeZone_, _instant_, _calendar_).
1. Let _after1_ be ? AddZonedDateTime(_relativeTo_.[[Nanoseconds]], _timeZone_, _calendar_, _one_.[[Years]], _one_.[[Months]], _one_.[[Weeks]], _one_.[[Days]], _one_.[[Hours]], _one_.[[Minutes]], _one_.[[Seconds]], _one_.[[Milliseconds]], _one_.[[Microseconds]], _one_.[[Nanoseconds]], _precalculatedDateTime_).
1. Let _after2_ be ? AddZonedDateTime(_relativeTo_.[[Nanoseconds]], _timeZone_, _calendar_, _two_.[[Years]], _two_.[[Months]], _two_.[[Weeks]], _two_.[[Days]], _two_.[[Hours]], _two_.[[Minutes]], _two_.[[Seconds]], _two_.[[Milliseconds]], _two_.[[Microseconds]], _two_.[[Nanoseconds]], _precalculatedDateTime_).
1. If _after1_ &gt; _after2_, return *1*<sub>𝔽</sub>.
1. If _after1_ &lt; _after2_, return *-1*<sub>𝔽</sub>.
1. Return *+0*<sub>𝔽</sub>.
1. If _calendarUnitsPresent_ is *true*, then
1. Assert: _relativeTo_ is either *undefined* or an Object with an [[InitializedTemporalDate]] internal slot.
1. Let _unbalanceResult1_ be ? UnbalanceDateDurationRelative(_one_.[[Years]], _one_.[[Months]], _one_.[[Weeks]], _one_.[[Days]], *"day"*, _relativeTo_).
1. Let _unbalanceResult2_ be ? UnbalanceDateDurationRelative(_two_.[[Years]], _two_.[[Months]], _two_.[[Weeks]], _two_.[[Days]], *"day"*, _relativeTo_).
1. Let _days1_ be _unbalanceResult1_.[[Days]].
1. Let _days2_ be _unbalanceResult2_.[[Days]].
1. Else,
1. Let _days1_ be _one_.[[Days]].
1. Let _days2_ be _two_.[[Days]].
1. Let _ns1_ be ! TotalDurationNanoseconds(_days1_, _one_.[[Hours]], _one_.[[Minutes]], _one_.[[Seconds]], _one_.[[Milliseconds]], _one_.[[Microseconds]], _one_.[[Nanoseconds]], _shift1_).
1. Let _ns2_ be ! TotalDurationNanoseconds(_days2_, _two_.[[Hours]], _two_.[[Minutes]], _two_.[[Seconds]], _two_.[[Milliseconds]], _two_.[[Microseconds]], _two_.[[Nanoseconds]], _shift2_).
1. Let _ns1_ be TotalDurationNanoseconds(_days1_, _one_.[[Hours]], _one_.[[Minutes]], _one_.[[Seconds]], _one_.[[Milliseconds]], _one_.[[Microseconds]], _one_.[[Nanoseconds]]).
1. Let _ns2_ be TotalDurationNanoseconds(_days2_, _two_.[[Hours]], _two_.[[Minutes]], _two_.[[Seconds]], _two_.[[Milliseconds]], _two_.[[Microseconds]], _two_.[[Nanoseconds]]).
1. If _ns1_ &gt; _ns2_, return *1*<sub>𝔽</sub>.
1. If _ns1_ &lt; _ns2_, return *-1*<sub>𝔽</sub>.
1. Return *+0*<sub>𝔽</sub>.
Expand Down Expand Up @@ -1138,31 +1148,6 @@ <h1>
</emu-alg>
</emu-clause>

<emu-clause id="sec-temporal-calculateoffsetshift" type="abstract operation">
<h1>
CalculateOffsetShift (
_relativeTo_: *undefined*, a Temporal.PlainDate, or a Temporal.ZonedDateTime,
_y_: an integer,
_mon_: an integer,
_w_: an integer,
_d_: an integer,
)
</h1>
<dl class="header">
<dt>description</dt>
<dd>It returns an integer difference in nanoseconds between the time zone offset at the time of _relativeTo_, and the time zone offset at the time of _relativeTo_ plus the given duration.</dd>
</dl>
<emu-alg>
1. If Type(_relativeTo_) is not Object or _relativeTo_ does not have an [[InitializedTemporalZonedDateTime]] internal slot, return 0.
1. Let _instant_ be ! CreateTemporalInstant(_relativeTo_.[[Nanoseconds]]).
1. Let _offsetBefore_ be ? GetOffsetNanosecondsFor(_relativeTo_.[[TimeZone]], _instant_).
1. Let _after_ be ? AddZonedDateTime(_relativeTo_.[[Nanoseconds]], _relativeTo_.[[TimeZone]], _relativeTo_.[[Calendar]], _y_, _mon_, _w_, _d_, 0, 0, 0, 0, 0, 0).
1. Let _instantAfter_ be ! CreateTemporalInstant(_after_).
1. Let _offsetAfter_ be ? GetOffsetNanosecondsFor(_relativeTo_.[[TimeZone]], _instantAfter_).
1. Return _offsetAfter_ - _offsetBefore_.
</emu-alg>
</emu-clause>

<emu-clause id="sec-temporal-totaldurationnanoseconds" type="abstract operation">
<h1>
TotalDurationNanoseconds (
Expand All @@ -1173,16 +1158,16 @@ <h1>
_milliseconds_: an integer,
_microseconds_: an integer,
_nanoseconds_: an integer,
_offsetShift_: an integer,
)
): an integer
</h1>
<dl class="header">
<dt>description</dt>
<dd>It computes an integer number of nanoseconds from the given units, applying a given time zone offset shift in nanoseconds when converting from days to hours.</dd>
<dd>
It computes an integer number of nanoseconds from the given units.
Note that this operation should not be used with _days_ &ne; 0 if the calculation is relative to a Temporal.ZonedDateTime.
</dd>
</dl>
<emu-alg>
1. If _days_ &ne; 0, then
1. Set _nanoseconds_ to _nanoseconds_ - _offsetShift_.
1. Set _hours_ to _hours_ + _days_ &times; 24.
1. Set _minutes_ to _minutes_ + _hours_ &times; 60.
1. Set _seconds_ to _seconds_ + _minutes_ &times; 60.
Expand Down Expand Up @@ -1236,7 +1221,7 @@ <h1>
<dd>It converts the time units of a duration into a form where lower units are converted into higher units as much as possible, up to _largestUnit_. If the Number value for any unit is infinite, it returns a special value indicating the direction of overflow.</dd>
</dl>
<emu-alg>
1. Set _nanoseconds_ to ! TotalDurationNanoseconds(_days_, _hours_, _minutes_, _seconds_, _milliseconds_, _microseconds_, _nanoseconds_, 0).
1. Set _nanoseconds_ to TotalDurationNanoseconds(_days_, _hours_, _minutes_, _seconds_, _milliseconds_, _microseconds_, _nanoseconds_).
1. Set _days_, _hours_, _minutes_, _seconds_, _milliseconds_, and _microseconds_ to 0.
1. If _nanoseconds_ &lt; 0, let _sign_ be -1; else, let _sign_ be 1.
1. Set _nanoseconds_ to abs(_nanoseconds_).
Expand Down Expand Up @@ -1730,7 +1715,7 @@ <h1>
1. If _unit_ is *"year"*, *"month"*, or *"week"*, and _plainRelativeTo_ is *undefined*, then
1. Throw a *RangeError* exception.
1. If _unit_ is one of *"year"*, *"month"*, *"week"*, or *"day"*, then
1. Let _nanoseconds_ be ! TotalDurationNanoseconds(0, _hours_, _minutes_, _seconds_, _milliseconds_, _microseconds_, _nanoseconds_, 0).
1. Let _nanoseconds_ be TotalDurationNanoseconds(0, _hours_, _minutes_, _seconds_, _milliseconds_, _microseconds_, _nanoseconds_).
1. If _zonedRelativeTo_ is not *undefined*, then
1. Let _intermediate_ be ? MoveRelativeZonedDateTime(_zonedRelativeTo_, _years_, _months_, _weeks_, _days_).
1. Let _result_ be ? NanosecondsToDays(_nanoseconds_, _intermediate_).
Expand Down Expand Up @@ -1893,7 +1878,7 @@ <h1>
<emu-alg>
1. If _unit_ is one of *"year"*, *"month"*, *"week"*, or *"day"*, then
1. Return ! CreateDurationRecord(_years_, _months_, _weeks_, _days_, _hours_, _minutes_, _seconds_, _milliseconds_, _microseconds_, _nanoseconds_).
1. Let _timeRemainderNs_ be ! TotalDurationNanoseconds(0, _hours_, _minutes_, _seconds_, _milliseconds_, _microseconds_, _nanoseconds_, 0).
1. Let _timeRemainderNs_ be TotalDurationNanoseconds(0, _hours_, _minutes_, _seconds_, _milliseconds_, _microseconds_, _nanoseconds_).
1. If _timeRemainderNs_ = 0, let _direction_ be 0.
1. Else if _timeRemainderNs_ &lt; 0, let _direction_ be -1.
1. Else, let _direction_ be 1.
Expand Down

0 comments on commit e34ef5b

Please sign in to comment.