1731620 - Unify DateTime.cpp in SpiderMonkey

Status: RESOLVED FIXED

(Core :: Internationalization, task)

Description

[André Bargull [:anba]]

https://searchfox.org/mozilla-central/source/js/src/vm/DateTime.cpp uses the ICU C++ TimeZone API, because some functionality wasn't initially available in the ICU C-API. Later ICU versions have added more time zones functions to the C-API, but only made them indirectly available through UCalendar. This means whenever any time zone offsets have to be computed, you first have to set the time on a UCalendar, which triggers that all calendar fields are recomputed.

Time zone offset calculations happen for all non-UTC methods on JavaScript's Date object, so we should avoid to introduce performance regressions if possible.

Here's a simple test case which requires 60% more time when using the ICU C-API instead of the C++ API:

function f() {
  // Use four entries to ensure we don't hit the offset cache in vm/DateTime.
  var xs = [
    Date.UTC(2010, 0, 1),
    Date.UTC(2010, 7, 1),
    Date.UTC(2020, 0, 1),
    Date.UTC(2020, 7, 1),
  ];

  var q = 0;
  var t = dateNow();
  for (var i = 0; i < 1_000_000; ++i) {
    var d = new Date(xs[i & 3]);
    q += d.getHours();
  }
  print(dateNow() - t, q);
}

Comment 1

[André Bargull [:anba]]

Attachment: Bug 1731620 - Part 1: Add more time zone functions to mozilla::intl::TimeZone. r=#platform-i18n-reviewers!

Add four additional methods to mozilla::intl::TimeZone:

1. GetDSTOffsetMs() to return the daylight saving offset at a specific UTC time.
2. GetOffsetMs() to return the time zone offset at a specific UTC time.
3. GetUTCOffsetMs() to return the UTC offset at a specific local time.
4. GetDisplayName() to return the display name of a time zone.

All four methods will be used to replace ICU calls in "js/src/vm/DateTime.cpp".

Comment 2

[André Bargull [:anba]]

Attachment: Bug 1731620 - Part 2: Use ICU C++ TimeZone API if available. r=#platform-i18n-reviewers!

ICU doesn't provide a C-API for time zone functions, but instead requires to
use UCalendar. This adds a noticeable overhead, because whenever time zone
offsets are computed, it's first necessary to set the time on a UCalendar,
which triggers a recomputation of all calendar fields. And because time zone
offset computation is used for JavaScript's Date, which is widely used, we
should avoid performance regressions compared to the current code which is
using the ICU C++ API.

We can only safely use the ICU C++ API when we don't use the system ICU,
because C++ doesn't have a stable ABI, so the ICU C++ API code paths are only
taken when MOZ_SYSTEM_ICU is false.

Depends on D126191

Comment 3

[André Bargull [:anba]]

Attachment: Bug 1731620 - Part 3: Make FormatBuffer's allocation policy configurable. r=#platform-i18n-reviewers!

Make FormatBuffer's allocation policy configurable, so we can it use with
js::SystemAllocPolicy in the next patch. This implies that the JSContext*
member has to be removed, too.

Depends on D126192

Comment 4

[André Bargull [:anba]]

Attachment: Bug 1731620 - Part 4: Change DateTime to work with mozilla::intl::TimeZone. r=#platform-i18n-reviewers!

Updates DateTime to use mozilla::intl::TimeZone for all time zone computations
when JS_HAS_INTL_API is defined. We no longer have to check !MOZ_SYSTEM_ICU,
because mozilla::intl::TimeZone supports ICU's C++ and C-API.

Depends on D126193

Comment 5

[André Bargull [:anba]]

Attachment: Bug 1731620 - Part 5: Use ICU for jsdate even when MOZ_SYSTEM_ICU is set. r=#platform-i18n-reviewers!

Now that DateTime works even when MOZ_SYSTEM_ICU is set, we no longer have
to check for MOZ_SYSTEM_ICU in "jsdate.cpp". This also allows to remove the
MOZ_SYSTEM_ICU checks in "vm/Time", because those were only called from
"jsdate.cpp" when MOZ_SYSTEM_ICU was set.

Depends on D126194

Comment 6

[André Bargull [:anba]]

Attachment: Bug 1731620 - Part 6: Add default time zone setters to mozila::intl::TimeZone. r=#platform-i18n-reviewers!

Add setters to change ICU's default time zone.

1. SetDefaultTimeZone() sets the default time zone from a time zone identifier.
2. SetDefaultTimeZoneFromHostTimeZone() synchronizes the default time zone with
   the host system's time zone.

The two new methods will be used in the next part to replace ICU calls in
js::DateTimeInfo::internalResyncICUDefaultTimeZone().

Depends on D126195

Comment 7

[André Bargull [:anba]]

Attachment: Bug 1731620 - Part 7: Call mozilla::intl::TimeZone to resynchronize the default time zone. r=#platform-i18n-reviewers!

Use mozilla::intl::TimeZone instead of directly calling ICU to change ICU's
default time zone.

Depends on D126196

Comment 8

[André Bargull [:anba]]

Attachment: Bug 1731620 - Part 8: Add TimeZone::GetHostTimeZone(). r=#platform-i18n-reviewers!

Add a wrapper for ucal_getHostTimeZone to mozilla::intl::TimeZone. This
function is only used for the "testing" function framework.

Depends on D126197

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f335336b9979
Part 1: Add more time zone functions to mozilla::intl::TimeZone. r=platform-i18n-reviewers,gregtatum
https://hg.mozilla.org/integration/autoland/rev/141f647c4ba6
Part 2: Use ICU C++ TimeZone API if available. r=platform-i18n-reviewers,gregtatum
https://hg.mozilla.org/integration/autoland/rev/9cedac891a43
Part 3: Make FormatBuffer's allocation policy configurable. r=platform-i18n-reviewers,gregtatum
https://hg.mozilla.org/integration/autoland/rev/b772bfee17ff
Part 4: Change DateTime to work with mozilla::intl::TimeZone. r=platform-i18n-reviewers,gregtatum
https://hg.mozilla.org/integration/autoland/rev/340c85c600ce
Part 5: Use ICU for jsdate even when MOZ_SYSTEM_ICU is set. r=platform-i18n-reviewers,gregtatum
https://hg.mozilla.org/integration/autoland/rev/08b6b3a466a4
Part 6: Add default time zone setters to mozila::intl::TimeZone. r=platform-i18n-reviewers,gregtatum
https://hg.mozilla.org/integration/autoland/rev/178b42bb4fac
Part 7: Call mozilla::intl::TimeZone to resynchronize the default time zone. r=platform-i18n-reviewers,gregtatum
https://hg.mozilla.org/integration/autoland/rev/3fd688600030
Part 8: Add TimeZone::GetHostTimeZone(). r=platform-i18n-reviewers,gregtatum

Comment 10

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/f335336b9979
https://hg.mozilla.org/mozilla-central/rev/141f647c4ba6
https://hg.mozilla.org/mozilla-central/rev/9cedac891a43
https://hg.mozilla.org/mozilla-central/rev/b772bfee17ff
https://hg.mozilla.org/mozilla-central/rev/340c85c600ce
https://hg.mozilla.org/mozilla-central/rev/08b6b3a466a4
https://hg.mozilla.org/mozilla-central/rev/178b42bb4fac
https://hg.mozilla.org/mozilla-central/rev/3fd688600030