1331473 - Get the default time zone when resolving the DateTimeFormat properties

Status: RESOLVED DUPLICATE of bug 1333197

(Core :: JavaScript: Internationalization API, defect)

Description

[André Bargull [:anba]]

This should help for bug 1314354 and improves the following µ-benchmark by 10-15%:

---
var t = Date.now();
for (var i = 0; i < 100000; ++i) new Intl.DateTimeFormat();
print(Date.now() - t);
---

Open question: Is this spec-compliant? Consider the case when the time zone is changed between calls to InitializeDateTimeFormat and resolveDateTimeFormatInternals. 6.4.3 DefaultTimeZone() [1] is specified to return "the host environment’s current time zone", but strictly speaking we're already not spec-compliant because of the deferred initialization of the Intl object and its constructors. So adding another case where we defer calling DefaultTimeZone() may be acceptable.

The alternative is too save the unchecked result of intl_defaultTimeZone() and later compute the actual time zone from the original return value of intl_defaultTimeZone().

This should help bug 1314354, because the Intl object is already initialized during start-up (by 
LoginManagerContextMenu.jsm [2]), which means Intl.DateTimeFormat.prototype gets initialized, which in turn leads to executing js::SharedIntlData::ensureTimeZones. And js::SharedIntlData::ensureTimeZones requires us to load the time zone names from ICU.

[1] https://tc39.github.io/ecma402/#sec-defaulttimezone
[2] https://dxr.mozilla.org/mozilla-central/rev/8eaf154b385bbe0ff06155294ccf7962aa2d3324/toolkit/components/passwordmgr/LoginManagerContextMenu.jsm#22-23

Comment 1

[André Bargull [:anba]]

Attachment: bug1331473.patch

Comment 2

[Jeff Walden [:Waldo]]

Comment on attachment 8827421 [details] [diff] [review]
bug1331473.patch

Review of attachment 8827421 [details] [diff] [review]:
-----------------------------------------------------------------

I think I prefer the alternative described in comment 0.

As to the prototypes being lazy-created, I wonder if they could be made into plain objects now.  Probably worth a try fixing that and updating the spec, too.

Comment 3

[Jeff Walden [:Waldo]]

LoginManagerContextMenu.jsm could probably stand to lazily create its Intl.DateTimeFormat object, too.  No good reason it has to occur during startup.

Comment 4

[André Bargull [:anba]]

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #3)
> LoginManagerContextMenu.jsm could probably stand to lazily create its
> Intl.DateTimeFormat object, too.  No good reason it has to occur during
> startup.

I think fixing the startup performance of Intl should be our job, but OTOH creating a single Intl.DateTimeFormat object in LoginManagerContextMenu.jsm also means they keep using the old time zone even when the user has selected a new time zone.

Comment 5

[André Bargull [:anba]]

Filed bug 1332610 for LoginManagerContextMenu.jsm

Comment 6

[André Bargull [:anba]]

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #2)
> I think I prefer the alternative described in comment 0.

To make this work, I needed to call both intl_defaultTimeZone() and intl_defaultTimeZoneOffset() in InitializeDateTimeFormat() (otherwise it's not possible to reconstruct the default time zone). And having two JS->C++ calls instead of one obviously leads to a performance degradation for the test case in comment #0. To reduce the performance loss I had to modify ICU to speed-up ucal_getDefaultTimeZone and I had to create a new ICU method to retrieve the default time zone offset (so we don't need to create a UCalendar object in intl_defaultTimeZoneOffset). And at that point I stopped, because I think these changes exceed the complexity budget we should spent here.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4999d0f84b57
Part 2: Remove the undefined check in Intl object finalizers which was only needed for Intl prototypes. r=Waldo

Comment 9

[André Bargull [:anba]]

(In reply to Pulsebot from comment #8)
> Pushed by [e-mail]:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/4999d0f84b57
> Part 2: Remove the undefined check in Intl object finalizers which was only
> needed for Intl prototypes. r=Waldo

Commit message fail by me. :-/

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4999d0f84b57

Comment 11

[André Bargull [:anba]]

Reopening and resetting flags, commit message contains wrong bug number.

Comment 13

[André Bargull [:anba]]

(In reply to André Bargull from comment #6)
> (In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #2)
> > I think I prefer the alternative described in comment 0.
> 
> To make this work, I needed to call both intl_defaultTimeZone() and
> intl_defaultTimeZoneOffset() in InitializeDateTimeFormat() (otherwise it's
> not possible to reconstruct the default time zone).

And the call to intl_defaultTimeZone() is too expensive according to bug 1333197, comment #0, so we need to avoid calling it, which is another reason the alternative idea doesn't work out.