Node: Consider enforcing --experimental-worker flag until Atomics.wake is renamed Atomics.notify

Created on 8 Jun 2018  Â·  16Comments  Â·  Source: nodejs/node

At the May TC39 meeting, I proposed renaming Atomics.wake to Atomics.notify in order to better differentiate it from Atomics.wait: Renaming Atomics.wake (please review before continuing). With complete support from implementers, the committee agreed and set forth "next steps".

The "needs consensus" PR will not be up for approval until late July. If Node ships the new Worker API without a flag before that approval, developers will inevitably write programs that rely on Atomics.wake and the argument on which I predicated this opportunity for improvement will be struck down, ie. that no new code featuring Atomics.wake has been written since the Spectre/Meltdown mitigations.

Can we work together on this? That would be ❤️ 👍

worker

Most helpful comment

I’m totally on board with the rename.

Once the decision for a name is made, what I’d see Node doing is providing the new name immediately, and providing Atomics.wake (with a deprecation warning either immediately or in the next major version). How exactly that looks is probably going to depend on how V8 ends up implementing the name change.

The "needs consensus" PR will not be up for approval until late July. If Node ships the new Worker API without a flag before that approval.

I don’t think that’s realistic, it’s going to take (possibly a lot) longer than that. If nothing else, we can always block that on this issue.

All 16 comments

cc @addaleax

this seems reasonable, I'm always a fan of helping out tc39. I doubt we would have unflagged workers by then anyway.

I’m totally on board with the rename.

Once the decision for a name is made, what I’d see Node doing is providing the new name immediately, and providing Atomics.wake (with a deprecation warning either immediately or in the next major version). How exactly that looks is probably going to depend on how V8 ends up implementing the name change.

The "needs consensus" PR will not be up for approval until late July. If Node ships the new Worker API without a flag before that approval.

I don’t think that’s realistic, it’s going to take (possibly a lot) longer than that. If nothing else, we can always block that on this issue.

As someone working on the current renaming proposal, I appreciate your support @devsnek @addaleax!

Hey @rwaldron @leobalter - from another angle it would be absolutely fantastic if you took --experimental-worker for a spin and give Node criticism about how it works and whether or not it aligns with your vision.

Note that even if we _do_ ship workers unflagged (which I think we won't, now that you've asked and Anna and Gus both agreed and I agree with them) - they'd still be experimental and we'd still be fine with "breaking" Atomics.wake.

@benjamingr will do!

Once the decision for a name is made

This has already been agreed to: Atomics.notify

@rwaldron Does that mean that we could introduce that alias in Node right now? Or is it “just” the fixed name in the proposal and TC39 has yet to decide whether it’s going to be merged in the spec that way?

@addaleax sorry for the delay responding...

Does that mean that we could introduce that alias in Node right now?

That would be ideal, yes.

Or is it “just” the fixed name in the proposal and TC39 has yet to decide whether it’s going to be merged in the spec that way?

The change has consensus, but was made subject to the "consensus PR" process.

is this all that is needed? do we need to worry about function.name?

Object.defineProperty(Atomics, 'notify', {
  value: Atomics.wake,
  writable: true,
  enumerable: false,
  configurable: true,
});

@devsnek I assume we want to do this in NewContext() in node.cc to make sure it’s available that way in VM contexts as well

is this all that is needed?

Yes, that works 👍

do we need to worry about function.name?

My nitpicking spec conformance self says yes, but my pragmatic self says no ;)

@devsnek do you want to do this? if not, i’ll put something together today or tomorrow

@addaleax yeah I can take it.

Should this be closed now that there's an alias?

I think it can. We have Atomics.notify() and Atomics.wake() emits a warning.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

vsemozhetbyt picture vsemozhetbyt  Â·  3Comments

filipesilvaa picture filipesilvaa  Â·  3Comments

Icemic picture Icemic  Â·  3Comments

fanjunzhi picture fanjunzhi  Â·  3Comments

stevenvachon picture stevenvachon  Â·  3Comments