[esnext] ship String.prototype.padStart / String.prototype.padEnd

[esnext] ship String.prototype.padStart / String.prototype.padEnd

Enable the --harmony-string-padding flag by default

BUG=v8:4954
R=adamk@chromium.org, littledan@chromium.org

Committed: https://crrev.com/8352a0feaccfd9a19f3b38564ed2c0859dd6e3d5
Cr-Commit-Position: refs/heads/master@{#40060}

[caitp, 2016-09-30 13:20:49]

The CQ bit was checked by caitp@igalia.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-30 13:20:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[caitp, 2016-09-30 13:21:36]

Intent thread sent at https://groups.google.com/forum/#!topic/v8-dev/OslNxzqivyI

[commit-bot: I haz the power, 2016-09-30 13:48:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-30 13:48:10]

Dry run: This issue passed the CQ dry run.

[foolip, 2016-09-30 13:50:08]

foolip@chromium.org changed reviewers:
+ foolip@chromium.org

[foolip, 2016-09-30 13:50:09]

Non-owner LGTM, that was fast :)

[Dan Ehrenberg, 2016-09-30 19:00:20]

Could you run cctest with TSAN on and in release mode to check if this will run
into the same issue as async/await did? It's not in a trybot.

[Dan Ehrenberg, 2016-09-30 19:00:21]

Could you run cctest with TSAN on and in release mode to check if this will run
into the same issue as async/await did? It's not in a trybot.

[caitp, 2016-09-30 19:16:44]

On 2016/09/30 19:00:21, Dan Ehrenberg wrote:
> Could you run cctest with TSAN on and in release mode to check if this will
run
> into the same issue as async/await did? It's not in a trybot.

the linux64_tsan_rel bot doesn't cover this? its late here, but i might be able
to tommorrow at some point

[caitp, 2016-09-30 20:25:48]

On 2016/09/30 19:16:44, caitp wrote:
> On 2016/09/30 19:00:21, Dan Ehrenberg wrote:
> > Could you run cctest with TSAN on and in release mode to check if this will
> run
> > into the same issue as async/await did? It's not in a trybot.
> 
> the linux64_tsan_rel bot doesn't cover this? its late here, but i might be
able
> to tommorrow at some point

looks like it does fail in the same way.

[caitp, 2016-09-30 23:24:48]

The CQ bit was checked by caitp@igalia.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-30 23:24:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-30 23:26:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-30 23:26:44]

Dry run: Try jobs failed on following builders:
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/13831)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/10009)

[caitp, 2016-09-30 23:27:43]

https://codereview.chromium.org/2382193002/diff/20001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/2382193002/diff/20001/src/bootstrapper.cc#new...
src/bootstrapper.cc:3465: static const char* harmony_string_padding_natives[] =
{nullptr};
I feel like we should just remove the flag if this is the "solution" to this
problem (as it was in the async-await case) --- it's strange to have a
`--no-harmony-string-padding` option that doesn't do what it claims to do.

Probably ought to file a bug about fixing the TSAN issue properly.

[caitp, 2016-09-30 23:29:09]

The CQ bit was checked by caitp@igalia.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-30 23:29:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Dan Ehrenberg, 2016-10-01 00:01:17]

Find the bug at https://bugs.chromium.org/p/v8/issues/detail?id=5427

[commit-bot: I haz the power, 2016-10-01 00:04:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-01 00:04:42]

Dry run: This issue passed the CQ dry run.

[adamk, 2016-10-03 18:05:49]

On 2016/10/01 00:01:17, Dan Ehrenberg (OOO) wrote:
> Find the bug at https://bugs.chromium.org/p/v8/issues/detail?id=5427

I think the right thing is to fix that bug (following mvstanton's suggestion)
before trying to ship more flag-guarded features.

[caitp, 2016-10-04 10:24:47]

On 2016/10/03 18:05:49, adamk wrote:
> On 2016/10/01 00:01:17, Dan Ehrenberg (OOO) wrote:
> > Find the bug at https://bugs.chromium.org/p/v8/issues/detail?id=5427
> 
> I think the right thing is to fix that bug (following mvstanton's suggestion)
> before trying to ship more flag-guarded features.

It's not clear exactly what he's suggesting. It looks like this is the same
calldescriptors data race that Michael described in the bug, which should
already be solved? I dunno if we have a way to trace the locking/unlocking of
isolate parts to see if there's a lock missing somewhere (perhaps around a
Bootstrapper::CompileExperimentalNative() call?).

[adamk, 2016-10-04 21:10:59]

On 2016/10/04 10:24:47, caitp wrote:
> On 2016/10/03 18:05:49, adamk wrote:
> > On 2016/10/01 00:01:17, Dan Ehrenberg (OOO) wrote:
> > > Find the bug at https://bugs.chromium.org/p/v8/issues/detail?id=5427
> > 
> > I think the right thing is to fix that bug (following mvstanton's
suggestion)
> > before trying to ship more flag-guarded features.
> 
> It's not clear exactly what he's suggesting. It looks like this is the same
> calldescriptors data race that Michael described in the bug, which should
> already be solved? I dunno if we have a way to trace the locking/unlocking of
> isolate parts to see if there's a lock missing somewhere (perhaps around a
> Bootstrapper::CompileExperimentalNative() call?).

This is different than the CallDescriptor race, but it's analogous. The idea is
that the Isolate should store thread-specific arrays of registers for the load
and store calling conventions. I'm working on a patch now.

[adamk, 2016-10-06 18:52:33]

lgtm now that the tsan fix has landed

[caitp, 2016-10-06 18:53:00]

Patchset #2 (id:20001) has been deleted

[caitp, 2016-10-06 18:53:11]

Patchset #2 (id:40001) has been deleted

[caitp, 2016-10-06 19:19:35]

The CQ bit was checked by caitp@igalia.com

[commit-bot: I haz the power, 2016-10-06 19:19:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[caitp, 2016-10-06 19:19:50]

On 2016/10/06 18:52:33, adamk wrote:
> lgtm now that the tsan fix has landed

Thanks, nice work on that

[commit-bot: I haz the power, 2016-10-06 19:53:21]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-10-06 19:53:49]

Description was changed from

==========
[esnext] ship String.prototype.padStart / String.prototype.padEnd

Enable the --harmony-string-padding flag by default

BUG=v8:4954
R=adamk@chromium.org, littledan@chromium.org
==========

to

==========
[esnext] ship String.prototype.padStart / String.prototype.padEnd

Enable the --harmony-string-padding flag by default

BUG=v8:4954
R=adamk@chromium.org, littledan@chromium.org

Committed: https://crrev.com/8352a0feaccfd9a19f3b38564ed2c0859dd6e3d5
Cr-Commit-Position: refs/heads/master@{#40060}
==========

[commit-bot: I haz the power, 2016-10-06 19:53:50]

Patchset 1 (id:??) landed as
https://crrev.com/8352a0feaccfd9a19f3b38564ed2c0859dd6e3d5
Cr-Commit-Position: refs/heads/master@{#40060}

[Clemens Hammacher, 2016-10-07 08:39:12]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2398183003/ by clemensh@chromium.org.

The reason for reverting is: Causes several GC bugs, e.g.
https://build.chromium.org/p/client.v8.ports/builders/V8%20Linux%20-%20arm64%....