796523 - Periodically expire JS timezone cache

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Justin Lebar (not reading bugmail)]

Have we considered periodically expiring the JS timezone cache?

The browser's internal timezone is actually quite visible to the user.  For example, Gmail shows dates and times according to the browser's timezone.

For people who travel, I don't think it's actually too uncommon to change your machine's timezone without restarting your browser.

Can we just expire our timezone cache after N minutes?  Or maybe even more clever, could we expire the timezone cache every time a new compartment is created?  That would ensure that refreshing a page always gets us the latest timezone, and likely wouldn't affect benchmarks.

Ideally, of course, we'd write code for each of our platforms which notices when the system timezone changes, but that seems much harder.

Comment 1

[Justin Lebar (not reading bugmail)]

Attachment: Patch? v1

Picking a reviewer at random here, please forward to someone else if I didn't
choose well.

I'm not sure how to do a performance test of this (I've never written a JS
patch) to ensure that it doesn't affect benchmark.  Should I run SS locally, or
is Talos sufficient?  If I need to run something locally, should I do it in the
JS shell, or use the browser?  If I need to use the JS shell, are there
instructions somewhere?

Comment 2

[Luke Wagner [:luke]]

Comment on attachment 666845 [details] [diff] [review]
Patch? v1

Waldo added this cache so I think he should be the reviewer.

My own 2 cents is that, while compartment creation is infrequent, it seems hacky to dump in something completely unrelated.  (Our classic dumping ground for "somewhat frequently but not *too* frequently" actions is GC :)

Comment 3

[Justin Lebar (not reading bugmail)]

> (Our classic dumping ground for "somewhat frequently but not *too* frequently" actions 
> is GC :)

I considered that, but I didn't think it was quite right, since I want to ensure that if I refresh a page or navigate to a new one, the date cache is up-to-date.

I'd be OK dumping the cache both on GC and on compartment creation...

Comment 4

[Jeff Walden [:Waldo]]

Comment on attachment 666845 [details] [diff] [review]
Patch? v1

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

I still want that ideal situation where the OS notifies us and we don't even have to have an API to allow embedders to affirmatively clear date caches.  :-(  So I am basically indifferent to whether GC or compartment-creation time or both is a good time to do this; if you want an actual opinion about either or both being better, find someone else.

::: js/src/jscompartment.cpp
@@ +98,5 @@
> +     * compartment.  This ensures that the cache is always relatively fresh, but
> +     * shouldn't interfere with benchmarks which create tons of date objects
> +     * (unless they also create tons of iframes, which seems unlikely).
> +     */
> +    JS_ClearDateCaches(cx);

Use js_ClearDateCaches() here, please, not the public API.

Comment 5

[Justin Lebar (not reading bugmail)]

Since nobody seems particularly concerned about the performance implications of this, I went ahead and pushed it.  Anyway it's a lot easier to find perf regressions on inbound than on try.

https://hg.mozilla.org/integration/mozilla-inbound/rev/92af89fcb638

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/92af89fcb638

Comment 7

[David Bruant]

for dev-doc-needed see this message (and related thread) https://mail.mozilla.org/pipermail/es-discuss/2013-July/032066.html