New issue
Advanced search Search tips
Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
HW: ----
NextAction: ----
OS: ----
Priority: 2
Type: FeatureRequest

Blocking:
issue chromium:754910



Sign in to add a comment
link

Issue 7883: Rename Atomics.wake to Atomics.notify per ES standard change

Reported by waldron....@gmail.com, Jun 22 2018

Issue description

At the May 2018 TC39 meeting, consensus was reached to rename Atomics.wake to Atomics.notify (while the API is temporarily disabled). 

The spec change can be found here: https://github.com/tc39/ecma262/pull/1220
 

Comment 1 by gsat...@chromium.org, Jun 27 2018

Components: Language
Labels: -Type-Bug Priority-3 Type-FeatureRequest
Owner: binji@chromium.org
Status: Assigned (was: Untriaged)

Comment 2 by hablich@chromium.org, Jul 11 2018

Blocking: chromium:754910

Comment 3 by bugdroid1@chromium.org, Jul 19 2018

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

commit c79206b36362d455f289bfea3689018e68dbfe41
Author: Ben Smith <binji@chromium.org>
Date: Thu Jul 19 00:14:29 2018

Add Atomics.notify as alias for Atomics.wake

At the May 2018 TC39 meeting, they decided to rename Atomics.wake to
Atomics.notify. This change adds Atomics.notify as an alias, but does
not remove Atomics.wake, which will be removed later.

This allows for embedders to use either name to prevent
breaking tests. When the tests are switched over, we can remove
Atomics.wake.

Bug: v8:7883
Change-Id: If057ebff162bde975c6e1b60d83a4662f144e81f
Reviewed-on: https://chromium-review.googlesource.com/1142290
Commit-Queue: Ben Smith <binji@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54534}
[modify] https://crrev.com/c79206b36362d455f289bfea3689018e68dbfe41/src/bootstrapper.cc
[modify] https://crrev.com/c79206b36362d455f289bfea3689018e68dbfe41/test/mjsunit/harmony/futex.js

Comment 4 by hablich@chromium.org, Jul 19 2018

Labels: Merge-Approved-6.9
Can you please merge this to 6.9?

Comment 5 by hablich@chromium.org, Jul 19 2018

Labels: Merge-Approved-6.8
Please also merge to 6.8, depending on the decision being made.

Comment 6 by bugdroid1@chromium.org, Jul 19 2018

Project Member
Labels: merge-merged-6.9
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/c80260179f5fbd4afd9412af1a43b2280547bcd9

commit c80260179f5fbd4afd9412af1a43b2280547bcd9
Author: Ben Smith <binji@chromium.org>
Date: Thu Jul 19 22:29:19 2018

Merged: Squashed multiple commits.

Merged: Add Atomics.notify as alias for Atomics.wake
Revision: c79206b36362d455f289bfea3689018e68dbfe41

Merged: [Atomics] Workaround for d8 worker limit
Revision: 6525dd1859301069cf47e0aec0002b307e856521

BUG= v8:7338 ,v8:7883
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=adamk@chromium.org

Change-Id: I3123edf45bd972087d76ed75fa1cac81fd670c9a
Reviewed-on: https://chromium-review.googlesource.com/1144185
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.9@{#5}
Cr-Branched-From: d7b61abe7b48928aed739f02bf7695732d359e7e-refs/heads/6.9.427@{#1}
Cr-Branched-From: b7e108d6016bf6b7de3a34e6d61cb522f5193460-refs/heads/master@{#54504}
[modify] https://crrev.com/c80260179f5fbd4afd9412af1a43b2280547bcd9/src/bootstrapper.cc
[modify] https://crrev.com/c80260179f5fbd4afd9412af1a43b2280547bcd9/test/mjsunit/harmony/futex.js

Comment 7 by bugdroid1@chromium.org, Jul 19 2018

Project Member
Labels: merge-merged-6.8
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/a9086aeb74a9c5ed9bebddae5b377f1fb147eb28

commit a9086aeb74a9c5ed9bebddae5b377f1fb147eb28
Author: Ben Smith <binji@chromium.org>
Date: Thu Jul 19 23:21:25 2018

Merged: Squashed multiple commits.

Merged: Add Atomics.notify as alias for Atomics.wake
Revision: c79206b36362d455f289bfea3689018e68dbfe41

Merged: [Atomics] Workaround for d8 worker limit
Revision: 6525dd1859301069cf47e0aec0002b307e856521

BUG= v8:7338 ,v8:7883
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=adamk@chromium.org

Change-Id: Ib9ea5c3fe3ec035526280973baa6a33fe49dc811
Reviewed-on: https://chromium-review.googlesource.com/1144450
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.8@{#48}
Cr-Branched-From: 44d7d7d6b1041b57644400a00cb3fee35f6c51b2-refs/heads/6.8.275@{#1}
Cr-Branched-From: 5754f66f75136dc17b4c63fec84f31dfdb89186e-refs/heads/master@{#53286}
[modify] https://crrev.com/a9086aeb74a9c5ed9bebddae5b377f1fb147eb28/src/bootstrapper.cc
[modify] https://crrev.com/a9086aeb74a9c5ed9bebddae5b377f1fb147eb28/test/mjsunit/harmony/futex.js

Comment 8 by gsat...@chromium.org, Sep 13

Labels: -Priority-3 Priority-2
Bumping this to P2. 

Ben said he'd add use counters to get usage numbers for Atomics.wake so that we can send out an Intent to Deprecate email soon.

Comment 9 by gsat...@chromium.org, Sep 13

Cc: binji@chromium.org
Owner: gsat...@chromium.org
Just talked a Ben, as he's short on cycles, I'm going to work on adding the use counters. Re-assigning to myself.

Comment 10 by bugdroid1@chromium.org, Sep 20

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/81c9e3936beaf761f2b57f879766b9aed875ed05

commit 81c9e3936beaf761f2b57f879766b9aed875ed05
Author: Sathya Gunasekaran <gsathya@chromium.org>
Date: Thu Sep 20 21:31:33 2018

[Atomics] Add use counter for Atomics.{wake, notify}

Previously, Atomics.notify was just an alias to Atomics.wake, which
doesn't quite let us add a use counter for these individual builtins.

This patch refactors the existing Atomics.wake into a separate
function that is called from two separate builtins.

Bug: v8:7883
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: If54c8f769b7949d88d327cfb2f70db394f32a0b7
Reviewed-on: https://chromium-review.googlesource.com/1234581
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Ben Smith <binji@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56105}
[modify] https://crrev.com/81c9e3936beaf761f2b57f879766b9aed875ed05/include/v8.h
[modify] https://crrev.com/81c9e3936beaf761f2b57f879766b9aed875ed05/src/bootstrapper.cc
[modify] https://crrev.com/81c9e3936beaf761f2b57f879766b9aed875ed05/src/builtins/builtins-definitions.h
[modify] https://crrev.com/81c9e3936beaf761f2b57f879766b9aed875ed05/src/builtins/builtins-sharedarraybuffer.cc
[modify] https://crrev.com/81c9e3936beaf761f2b57f879766b9aed875ed05/test/cctest/test-usecounters.cc
[add] https://crrev.com/81c9e3936beaf761f2b57f879766b9aed875ed05/test/mjsunit/harmony/atomics-notify.js

Comment 11 by bugdroid1@chromium.org, Oct 2

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/76ed477272a64b5895a41b27b0e9d4e1b35142f9

commit 76ed477272a64b5895a41b27b0e9d4e1b35142f9
Author: Sathya Gunasekaran <gsathya@chromium.org>
Date: Tue Oct 02 03:22:41 2018

Plumb through use counter from V8 for Atomics.{wake, notify}

Atomics.notify is the new alias for Atomics.wake. This use counter
lets us figure out if Atomics.wake can be deprecated without
breakage.

V8 use counters were added here:
https://chromium-review.googlesource.com/c/v8/v8/+/1234581

Bug: v8:7883
Change-Id: I8eb7f32535aa3d913362aae1544e1dc2f595d38e
Reviewed-on: https://chromium-review.googlesource.com/1255587
Reviewed-by: Ben Smith <binji@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595713}
[modify] https://crrev.com/76ed477272a64b5895a41b27b0e9d4e1b35142f9/third_party/blink/public/platform/web_feature.mojom
[modify] https://crrev.com/76ed477272a64b5895a41b27b0e9d4e1b35142f9/third_party/blink/renderer/bindings/core/v8/use_counter_callback.cc
[modify] https://crrev.com/76ed477272a64b5895a41b27b0e9d4e1b35142f9/tools/metrics/histograms/enums.xml

Comment 12 by gsat...@chromium.org, Oct 2

Cc: -binji@chromium.org gsat...@chromium.org
Owner: binji@chromium.org
Re-assigning to binji for the Intent to Deprecate email

Comment 13 by ofrobots@google.com, Dec 13

Labels: NodeJS-Backport-Rejected

Comment 14 by gsat...@chromium.org, Dec 18

Looks like we have some numbers from the use counters: 
Atomics.wake: https://www.chromestatus.com/metrics/feature/timeline/popularity/2556
Atomics.notify: https://www.chromestatus.com/metrics/feature/timeline/popularity/2555

Seems small enough to deprecate. Ben, can you send out the intent to deprecate email?

Sign in to add a comment