All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=769706

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=89c4428f669fa4f4134bc0103fb5fbfe529755f0ddffc9a46685ffd514e29655


Bot(s) for this bug's original alert(s):

Atom_x64
Haswell_x64
ia32
x64

What's the best way to run this test locally?  

This must be caused by switching to ICU for timezone.  

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/28ef8dc7009d57de2782668ed00dff34e31dee2d

commit 28ef8dc7009d57de2782668ed00dff34e31dee2d
Author: Jungshik Shin <jshin@chromium.org>
Date: Wed Oct 11 23:21:37 2017

Revert "Enable icu-timezone-data by default"

This reverts commit d9a25842d3aba82ead5e06c7fd7cb4896e6ad202.

Reason for revert: 

I'm reverting this CL for a few reasons. #2 is the most significant and I should have thought of that before making a switch. Sorry for that.   

1) perf-regression:  http://crbug.com/769706 
2)  http://crbug.com/612010  : ICU timezone update is not propagated to zygote process so that new tabs will hold on to an old timezone even after a timezone change on Linux and Chrome OS. 
3)  http://crbug.com/754053  : OS timezone detection issues on macOS 10.13, Ubutu 16, RHEL 7, SuSe Linux 12 or newer. ; it's being fixed. So, it actually ok.  
4)  http://crbug.com/771868  : timezone wrong in gmail: If it's due to #3, we're fine because it's fixed. If not, we need to look more.

Original change's description:
> Enable icu-timezone-data by default
> 
> This will introduce a new behavior on POSIX(-like) platforms. Timezone
> names inside parentheses after GMT offset will not be 3-4 letter
> abbreviation any longer. They'll be human-readable names in the current
> default locale. This matches the current Windows behavior.
> 
> new Date(2017, 5, 22).toString()
> new Date(2017, 11, 22).toString()
> 
> Current:
> 
> Thu Jun 22 2017 00:00:00 GMT-0700 (PDT)
> Fri Dec 22 2017 00:00:00 GMT-0800 (PST)
> 
> New in en-US locale:
> 
> Thu Jun 22 2017 00:00:00 GMT-0700 (Pacific Daylight Time)
> Fri Dec 22 2017 00:00:00 GMT-0800 (Pacific Standard Time)
> 
> New in German locale:
> 
> Thu Jun 22 2017 00:00:00 GMT-0700 (Nordamerikanische Westküsten-Sommerzeit)
> Fri Dec 22 2017 00:00:00 GMT-0800 (Nordamerikanische Westküsten-Normalzeit)
> 
> BUG= v8:6031 ,  v8:2137 , v8:6076
> TEST=mjsunit/icu-date-lord-howe.js, mjsunit/icu-date-to-string.js
> 
> Change-Id: I4e7fd8b3ddae5c7779e220c4c101e45904fcdc01
> Reviewed-on: https://chromium-review.googlesource.com/625164
> Commit-Queue: Jungshik Shin <jshin@chromium.org>
> Reviewed-by: Daniel Ehrenberg <littledan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#47953}

TBR=adamk@chromium.org,littledan@chromium.org,jshin@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  v8:6031 ,  v8:2137 , v8:6076,  chromium:769706 ,  chromium:612010 ,  chromium:771868 
Change-Id: I60d75467ee21975d3a235344b01c0d2d44a7da96
Reviewed-on: https://chromium-review.googlesource.com/713404
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48478}
[modify] https://crrev.com/28ef8dc7009d57de2782668ed00dff34e31dee2d/src/flag-definitions.h

Perhaps, I can do something similar to what I did with  bug 766816 . 
That is, run perf tests on perf trybots with a CL to enable ICU-timezone-data and do the A-B test. 

Would that work for v8 tests?  Hmm... v8 checkout does not have  tools/perf/run_benchmark try .  

If possible, I want to run benchmark on perf bots instead of on my local machine to avoid noise as much as possible. 

If that's not possible, I'll try running it locally. That would let me find hot spots if any. 

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/d7343703bc6a3c1dbd5ead201186c520ffc9e47b

commit d7343703bc6a3c1dbd5ead201186c520ffc9e47b
Author: Jungshik Shin <jshin@chromium.org>
Date: Mon Oct 16 20:43:16 2017

Merged: Revert "Enable icu-timezone-data by default"

This reverts commit d9a25842d3aba82ead5e06c7fd7cb4896e6ad202.
This is for 6.3 branch (to go with Chromium 63 branch). 

Reason for revert:

I'm reverting this CL for a few reasons. #2 is the most significant and
I should have thought of that before making a switch. Sorry for that.

1) perf-regression:  http://crbug.com/769706 
2)  http://crbug.com/612010  : ICU timezone update is not propagated to
zygote process so that new tabs will hold on to an old timezone even
after a timezone change on Linux and Chrome OS.

3)   http://crbug.com/754053   : OS timezone detection issues on macOS
10.13, Ubutu 16, RHEL 7, SuSe Linux 12 or newer. ; it's being fixed. So,
it actually ok.

4)   http://crbug.com/771868   : timezone wrong in gmail: If it's due to

Revision: 28ef8dc7009d

BUG= chromium:612010 , chromium:769706 , chromium:771868 , v8:2137 , v8:6031 ,v8:6076
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=adamk@chromium.org, hablich@chromium.org

Change-Id: Id0093c2bc69e5bc1d3b7147b7bbcc633ed625a45
Reviewed-on: https://chromium-review.googlesource.com/721919
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.3@{#7}
Cr-Branched-From: 094a7c93dcdcd921de3883ba4674b7e1a0feffbe-refs/heads/6.3.292@{#1}
Cr-Branched-From: 18b8fbb528a8021e04a029e06eafee50b918bce0-refs/heads/master@{#48432}
[modify] https://crrev.com/d7343703bc6a3c1dbd5ead201186c520ffc9e47b/src/flag-definitions.h

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/3b1cfa38bcd616609f263ac3b6721c0214c45d19

commit 3b1cfa38bcd616609f263ac3b6721c0214c45d19
Author: Michael Hablich <hablich@chromium.org>
Date: Tue Oct 17 16:11:43 2017

Revert "Merged: Revert "Enable icu-timezone-data by default""

This reverts commit d7343703bc6a3c1dbd5ead201186c520ffc9e47b.

Reason for revert: broke some branch builders like https://build.chromium.org/p/client.v8.branches/builders/V8%20arm%20-%20sim%20-%20beta%20branch%20-%20debug

Original change's description:
> Merged: Revert "Enable icu-timezone-data by default"
> 
> This reverts commit d9a25842d3aba82ead5e06c7fd7cb4896e6ad202.
> This is for 6.3 branch (to go with Chromium 63 branch). 
> 
> Reason for revert:
> 
> I'm reverting this CL for a few reasons. #2 is the most significant and
> I should have thought of that before making a switch. Sorry for that.
> 
> 1) perf-regression:  http://crbug.com/769706 
> 2)  http://crbug.com/612010  : ICU timezone update is not propagated to
> zygote process so that new tabs will hold on to an old timezone even
> after a timezone change on Linux and Chrome OS.
> 
> 3)   http://crbug.com/754053   : OS timezone detection issues on macOS
> 10.13, Ubutu 16, RHEL 7, SuSe Linux 12 or newer. ; it's being fixed. So,
> it actually ok.
> 
> 4)   http://crbug.com/771868   : timezone wrong in gmail: If it's due to
> 
> Revision: 28ef8dc7009d
> 
> BUG= chromium:612010 , chromium:769706 , chromium:771868 , v8:2137 , v8:6031 ,v8:6076
> LOG=N
> NOTRY=true
> NOPRESUBMIT=true
> NOTREECHECKS=true
> R=​adamk@chromium.org, hablich@chromium.org
> 
> Change-Id: Id0093c2bc69e5bc1d3b7147b7bbcc633ed625a45
> Reviewed-on: https://chromium-review.googlesource.com/721919
> Reviewed-by: Adam Klein <adamk@chromium.org>
> Commit-Queue: Jungshik Shin <jshin@chromium.org>
> Cr-Commit-Position: refs/branch-heads/6.3@{#7}
> Cr-Branched-From: 094a7c93dcdcd921de3883ba4674b7e1a0feffbe-refs/heads/6.3.292@{#1}
> Cr-Branched-From: 18b8fbb528a8021e04a029e06eafee50b918bce0-refs/heads/master@{#48432}

TBR=adamk@chromium.org,hablich@chromium.org,jshin@chromium.org

Change-Id: I8b30f8f9c63b93cb000facbf910ed111ca3f9ab2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:612010 ,  chromium:769706 ,  chromium:771868 ,  v8:2137 ,  v8:6031 , v8:6076
Reviewed-on: https://chromium-review.googlesource.com/723324
Reviewed-by: Michael Hablich <hablich@chromium.org>
Commit-Queue: Michael Hablich <hablich@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.3@{#18}
Cr-Branched-From: 094a7c93dcdcd921de3883ba4674b7e1a0feffbe-refs/heads/6.3.292@{#1}
Cr-Branched-From: 18b8fbb528a8021e04a029e06eafee50b918bce0-refs/heads/master@{#48432}
[modify] https://crrev.com/3b1cfa38bcd616609f263ac3b6721c0214c45d19/src/flag-definitions.h

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/9685d9c25a45f7902fe6250b64ca61abeea1e9f5

commit 9685d9c25a45f7902fe6250b64ca61abeea1e9f5
Author: Jungshik Shin <jshin@chromium.org>
Date: Tue Oct 17 20:55:47 2017

Revert "Revert "Merged: Revert "Enable icu-timezone-data by default"""

This reverts commit 3b1cfa38bcd616609f263ac3b6721c0214c45d19.

Reason for revert: ICU was not rolled in 6.3 branch leading invalid-locale test failure (that was added to test an ICU fix). Now, ICU is rolled in 6.3 branch ( https://chromium-review.googlesource.com/c/v8/v8/+/723564 ). 

Original change's description:
> Revert "Merged: Revert "Enable icu-timezone-data by default""
> 
> This reverts commit d7343703bc6a3c1dbd5ead201186c520ffc9e47b.
> 
> Reason for revert: broke some branch builders like https://build.chromium.org/p/client.v8.branches/builders/V8%20arm%20-%20sim%20-%20beta%20branch%20-%20debug
> 
> Original change's description:
> > Merged: Revert "Enable icu-timezone-data by default"
> > 
> > This reverts commit d9a25842d3aba82ead5e06c7fd7cb4896e6ad202.
> > This is for 6.3 branch (to go with Chromium 63 branch). 
> > 
> > Reason for revert:
> > 
> > I'm reverting this CL for a few reasons. #2 is the most significant and
> > I should have thought of that before making a switch. Sorry for that.
> > 
> > 1) perf-regression:  http://crbug.com/769706 
> > 2)  http://crbug.com/612010  : ICU timezone update is not propagated to
> > zygote process so that new tabs will hold on to an old timezone even
> > after a timezone change on Linux and Chrome OS.
> > 
> > 3)   http://crbug.com/754053   : OS timezone detection issues on macOS
> > 10.13, Ubutu 16, RHEL 7, SuSe Linux 12 or newer. ; it's being fixed. So,
> > it actually ok.
> > 
> > 4)   http://crbug.com/771868   : timezone wrong in gmail: If it's due to
> > 
> > Revision: 28ef8dc7009d
> > 
> > BUG= chromium:612010 , chromium:769706 , chromium:771868 , v8:2137 , v8:6031 ,v8:6076
> > LOG=N
> > NOTRY=true
> > NOPRESUBMIT=true
> > NOTREECHECKS=true
> > R=​adamk@chromium.org, hablich@chromium.org
> > 
> > Change-Id: Id0093c2bc69e5bc1d3b7147b7bbcc633ed625a45
> > Reviewed-on: https://chromium-review.googlesource.com/721919
> > Reviewed-by: Adam Klein <adamk@chromium.org>
> > Commit-Queue: Jungshik Shin <jshin@chromium.org>
> > Cr-Commit-Position: refs/branch-heads/6.3@{#7}
> > Cr-Branched-From: 094a7c93dcdcd921de3883ba4674b7e1a0feffbe-refs/heads/6.3.292@{#1}
> > Cr-Branched-From: 18b8fbb528a8021e04a029e06eafee50b918bce0-refs/heads/master@{#48432}
> 
> TBR=adamk@chromium.org,hablich@chromium.org,jshin@chromium.org
> 
> Change-Id: I8b30f8f9c63b93cb000facbf910ed111ca3f9ab2
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  chromium:612010 ,  chromium:769706 ,  chromium:771868 ,  v8:2137 ,  v8:6031 , v8:6076
> Reviewed-on: https://chromium-review.googlesource.com/723324
> Reviewed-by: Michael Hablich <hablich@chromium.org>
> Commit-Queue: Michael Hablich <hablich@chromium.org>
> Cr-Commit-Position: refs/branch-heads/6.3@{#18}
> Cr-Branched-From: 094a7c93dcdcd921de3883ba4674b7e1a0feffbe-refs/heads/6.3.292@{#1}
> Cr-Branched-From: 18b8fbb528a8021e04a029e06eafee50b918bce0-refs/heads/master@{#48432}

TBR=adamk@chromium.org,hablich@chromium.org,jshin@chromium.org

Change-Id: I8f712f1e6eb1f14c703ffe9f1d63d23b4b4bb08e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:612010 ,  chromium:769706 ,  chromium:771868 ,  v8:2137 ,  v8:6031 , v8:6076
Reviewed-on: https://chromium-review.googlesource.com/723962
Reviewed-by: Michael Hablich <hablich@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.3@{#21}
Cr-Branched-From: 094a7c93dcdcd921de3883ba4674b7e1a0feffbe-refs/heads/6.3.292@{#1}
Cr-Branched-From: 18b8fbb528a8021e04a029e06eafee50b918bce0-refs/heads/master@{#48432}
[modify] https://crrev.com/9685d9c25a45f7902fe6250b64ca61abeea1e9f5/src/flag-definitions.h

SunSpider/date-format-tofte  is a regressed test. 

tools/try_perf.py  is a tool to run perf tests on bots. 

I profiled the latest build of d8 with and without icu-timezone-data option running date-format-tofte.  Nothing pops up in the profiles.  

Is there a reason to R-V-G this issue?  I'll open it up unless I hear otherwise. 

hablich: this is R-V-G by devault because the performance tests run on the internal v8 waterfall. Can it be opened up per #12?

hablich:  is it ok to open this up?  (I got an email inquiry from an external user about this bug because he is eager to see  bug v8:6031  fixed). 

BTW, Adam:  what's your take on taking a perf hit for this one test ( SunSpider/date-format-tofte )  to fix  bug v8:6031  ?  

I'd love to find the cause, but haven't had much time to spend recently. 

This regression is big enough that I think we should understand why this causes performance problems before deciding how to move forward.

 Thank you for the reply, Adam.  I'll try to get to the bottom of the regression. 

BTW, the primary concern of a person who contacted me about  bug v8:6031  and this bug was  bug 520783  (Date's non-intl methods do not take into account the timezone change on Android). 

Let's remove the restricted label. Sunspider is not a private benchmark, so there should be no concern.

Thank you, Michael, for lifting the restriction. 

Without icu-timezone-data,  the timezone name is 3 or 4 letter abbrevition (e.g. EST, EDT, CET, EET, KST, etc) on platforms other than Windows. 
With icu-timezone-data, it's longer (US Eastern Standard Time, Central European Standard Time, Korean Standard Time, etc).   

Could it make this rather large ( 5 ~ 44 %) difference for SunSpider/date-format-tofte?  

Is Sunspider test also run on Windows on a regular basis?   If so, I'm interested to see whether there's a similar regression or not on Windows. 

If windows perf bot data is not available, I can change the behavior of icu-timezone-data to use a 3~4 letter abbreviations on non-Windows and run Sunspider locally to see the difference. 

We are not running Sunspider on Windows. mvstanton@ might be able to answer your question regarding Sunspider.

https://chromium-review.googlesource.com/c/v8/v8/+/599377 does what I wrote in comment 20 (use 3~4 letter abbreviations on "POSIX" (virtually all platforms other than Windows). 

I'll test it out. 

I ran two perf tests:

1. Enabling icu-timezone-data by default

2. Enabling icu-timezone-data by default and using a short timezone name (comment 23). 

Both perf trybot results are 

https://docs.google.com/spreadsheets/d/1SVq7XkNbvawTv7xGm_VMiHKvyLO74H5BFerCCbXiDcU/edit?usp=sharing

Somehow the test side (with a patch) has a rather larger warm-up time (run #1) on all the bots except for nexus 5 bots. For instance, linux64_atom, run#1 on the test-side took > 150ms while all the subsequent runs take about 60ms.
 


With run #1 removed, linux haswell has the largest (and rather clear) performance loss with icu-timezone-data enabled.  On nexus5, test and baseline are  kinda tie. On atom, linux64, linux32, test and baseline are about 2-sigmas away from each other. 

Forgot to say: long or short timezone names with icu-timezone-data makes very little difference. 

For the record, I ran the following with a CL enabling icu-timezone-data by default: 

./tools/try_perf.py --linux64_atom --arm32 --linux64_haswell --linux64 \
   --linux32 --nexus5   sunspider

I added p-value (with t-test) to the spreadsheet even though it's rather clear that there are differences between non-icu and icu for most bots (excluding run #1). %-diff is also added. It's 4% ~ 38% depending on bots. 

The warm-up time in run #1 is likely to be due to the disk scan (/usr/share/zoneinfo) to determine the current timezone (perhaps, starting with the 2nd run, disk cache(?) helps). In Chrome, we'd not have to pay for that because the default timezone is determined and handed down to Blink and v8 by a browser process. 

sunspider/date-format-totfe does not print out the timezone name at all. So, it makes sense that short or long timezone name makes little, if any, difference. 

It's likely that most (if not all) of differences in totfe between non-ICU and ICU come from the zone offset/DST offset calculation.  

ICU gives us the correct offset for all time zones (at least since 1970) while non-ICU does not for time zones with zone offset change history (see bugs blocked by  bug v8:6031  and  bug v8:3547  )

localtime_r() (handled with IPC across sandbox) in Chrome is likely to be slower than ICU (that does not use localtime_r()  for timezone offset calculation). To be fair, non-ICU implementation does cache zoneoffset and DST offset. 

Anyway, the difference between the two inside Chrome is likely to be smaller. 
I'll run sunspider/date-format-totfe inside Chrome with and without icu-timezone-data and see what kind of difference I get (well, it'd be rather noisy). 

From a performance sheriff perspective, I think the change to use ICU shouldn't be blocked on the disk read we see on the first run.

If results within Chrome are smaller (closer to a 12% regression than the +30% regression seen on Linux64 Haswell) then the move to ICU should go forward.

Thank you, mvstanton@, for the comment. 

I ran the test in Chrome on a couple of machines. 

On my corp MacBook Pro, two are almost identical, but ICU is slightly faster (I jotted down numbers, but left it at my desk). I'll add that information later.

On my desktop Linux, non-ICU one is faster (the difference is certainly less than 30%. I'll re-run it tomorrow and report back here)

I reran in-chrome test again on 4 different devices (
MacBook Pro 2015(?), Linux Dell Z8x0, Chromebook Pixel, Nexus 5X). 

The results are at :

https://docs.google.com/spreadsheets/d/1SVq7XkNbvawTv7xGm_VMiHKvyLO74H5BFerCCbXiDcU/edit#gid=1977858721

With our without icu-data-timezone,  the performance does not change at all. P-value  is over 0.5 (not even close to typical threshold of significant difference : 0.05, 0.01, etc). 

BTW, https://chromium-review.googlesource.com/890176 may have helped a little bit. 

I'll make a CL to re-enable icu-data-timezone.  

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/ae314567068815d494ef206db54a887cdd614db7

commit ae314567068815d494ef206db54a887cdd614db7
Author: Jungshik Shin <jshin@chromium.org>
Date: Wed Mar 07 18:09:43 2018

Re-enable icu-timezone-data by default

icu-timezone-data was enabled before but reverted due to a perf issue.
(sunspider/date-format-totfe regressed;  crbug.com/769706  ).

However, my in-Chrome test of the same test [1] shows that there's virtually
no perf difference. See https://goo.gl/GX1jt6 .

This will introduce a new behavior on POSIX(-like) platforms. Timezone
names inside parentheses after GMT offset will not be 3-4 letter
abbreviation any longer. They'll be human-readable names in the current
default locale. This matches the current Windows behavior.

new Date(2017, 5, 22).toString()
new Date(2017, 11, 22).toString()

Current:

Thu Jun 22 2017 00:00:00 GMT-0700 (PDT)
Fri Dec 22 2017 00:00:00 GMT-0800 (PST)

New:

Thu Jun 22 2017 00:00:00 GMT-0700 (Pacific Daylight Time)
Fri Dec 22 2017 00:00:00 GMT-0800 (Pacific Standard Time)

This CL will be followed by
  https://chromium-review.googlesource.com/c/v8/v8/+/572148 to
implement https://github.com/tc39/ecma262/pull/778 .

[1] http://jungshik.github.io/v8/cr769706.html

BUG= v8:6031 ,  v8:2137 , v8:6076,  chromium:769706 
TEST=mjsunit/icu-date-lord-howe.js, mjsunit/icu-date-to-string.js

Change-Id: I22203670c3307a57fbf99e5f0a271dcbfbbef8fd
Reviewed-on: https://chromium-review.googlesource.com/857333
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51791}
[modify] https://crrev.com/ae314567068815d494ef206db54a887cdd614db7/src/flag-definitions.h