Bug 1260835 - Atomics.wait returns strings + remove symbolic constants. r=jolesen
authorLars T Hansen <lhansen@mozilla.com>
Sat, 02 Apr 2016 08:41:28 -0700
changeset 291512 fd3ca174abe074fbde07dd3fb7efe1d5f5da477a
parent 291511 63bdfecc99f488142d1601f381f6241fd22ddb92
child 291513 823040191adc78b25f1c8ec6d0523c209b97598f
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjolesen
bugs1260835
milestone48.0a1
Bug 1260835 - Atomics.wait returns strings + remove symbolic constants. r=jolesen
js/src/builtin/AtomicsObject.cpp
js/src/builtin/AtomicsObject.h
js/src/tests/shell/futex.js
js/src/vm/CommonPropertyNames.h
--- a/js/src/builtin/AtomicsObject.cpp
+++ b/js/src/builtin/AtomicsObject.cpp
@@ -786,17 +786,17 @@ js::atomics_wait(JSContext* cx, unsigned
         return ReportCannotWait(cx);
 
     // This lock also protects the "waiters" field on SharedArrayRawBuffer,
     // and it provides the necessary memory fence.
     AutoLockFutexAPI lock;
 
     SharedMem<int32_t*>(addr) = view->viewDataShared().cast<int32_t*>() + offset;
     if (jit::AtomicOperations::loadSafeWhenRacy(addr) != value) {
-        r.setInt32(AtomicsObject::FutexNotequal);
+        r.setString(cx->names().futexNotEqual);
         return true;
     }
 
     Rooted<SharedArrayBufferObject*> sab(cx, view->bufferShared());
     SharedArrayRawBuffer* sarb = sab->rawBufferObject();
 
     FutexWaiter w(offset, rt);
     if (FutexWaiter* waiters = sarb->waiters()) {
@@ -804,20 +804,28 @@ js::atomics_wait(JSContext* cx, unsigned
         w.back = waiters->back;
         waiters->back->lower_pri = &w;
         waiters->back = &w;
     } else {
         w.lower_pri = w.back = &w;
         sarb->setWaiters(&w);
     }
 
-    AtomicsObject::FutexWaitResult result = AtomicsObject::FutexOK;
+    FutexRuntime::WaitResult result = FutexRuntime::FutexOK;
     bool retval = rt->fx.wait(cx, timeout_ms, &result);
-    if (retval)
-        r.setInt32(result);
+    if (retval) {
+        switch (result) {
+          case FutexRuntime::FutexOK:
+            r.setString(cx->names().futexOK);
+            break;
+          case FutexRuntime::FutexTimedOut:
+            r.setString(cx->names().futexTimedOut);
+            break;
+        }
+    }
 
     if (w.lower_pri == &w) {
         sarb->setWaiters(nullptr);
     } else {
         w.lower_pri->back = w.back;
         w.back->lower_pri = w.lower_pri;
         if (sarb->waiters() == &w)
             sarb->setWaiters(w.lower_pri);
@@ -949,17 +957,17 @@ js::FutexRuntime::isWaiting()
     // wakes up and goes into WaitingInterrupted.  In those states the
     // worker is still waiting, and if an explicit wake arrives the
     // worker transitions to Woken.  See further comments in
     // FutexRuntime::wait().
     return state_ == Waiting || state_ == WaitingInterrupted || state_ == WaitingNotifiedForInterrupt;
 }
 
 bool
-js::FutexRuntime::wait(JSContext* cx, double timeout_ms, AtomicsObject::FutexWaitResult* result)
+js::FutexRuntime::wait(JSContext* cx, double timeout_ms, WaitResult* result)
 {
     MOZ_ASSERT(&cx->runtime()->fx == this);
     MOZ_ASSERT(cx->runtime()->fx.canWait());
     MOZ_ASSERT(lockHolder_ == PR_GetCurrentThread());
     MOZ_ASSERT(state_ == Idle || state_ == WaitingInterrupted);
 
     // Disallow waiting when a runtime is processing an interrupt.
     // See explanation below.
@@ -1006,24 +1014,24 @@ js::FutexRuntime::wait(JSContext* cx, do
         lockHolder_ = holder;
 #endif
         switch (state_) {
           case FutexRuntime::Waiting:
             // Timeout or spurious wakeup.
             if (timed) {
                 uint64_t now = PRMJ_Now();
                 if (now >= finalEnd) {
-                    *result = AtomicsObject::FutexTimedout;
+                    *result = FutexTimedOut;
                     goto finished;
                 }
             }
             break;
 
           case FutexRuntime::Woken:
-            *result = AtomicsObject::FutexOK;
+            *result = FutexOK;
             goto finished;
 
           case FutexRuntime::WaitingNotifiedForInterrupt:
             // The interrupt handler may reenter the engine.  In that case
             // there are two complications:
             //
             // - The waiting thread is not actually waiting on the
             //   condition variable so we have to record that it
@@ -1054,17 +1062,17 @@ js::FutexRuntime::wait(JSContext* cx, do
             state_ = WaitingInterrupted;
             {
                 AutoUnlockFutexAPI unlock;
                 retval = cx->runtime()->handleInterrupt(cx);
             }
             if (!retval)
                 goto finished;
             if (state_ == Woken) {
-                *result = AtomicsObject::FutexOK;
+                *result = FutexOK;
                 goto finished;
             }
             break;
 
           default:
             MOZ_CRASH();
         }
     }
@@ -1111,39 +1119,30 @@ const JSFunctionSpec AtomicsMethods[] = 
     JS_INLINABLE_FN("isLockFree",         atomics_isLockFree,         1,0, AtomicsIsLockFree),
     JS_FN("wait",                         atomics_wait,               4,0),
     JS_FN("futexWait",                    atomics_wait,               4,0),
     JS_FN("wake",                         atomics_wake,               3,0),
     JS_FN("futexWake",                    atomics_wake,               3,0),
     JS_FS_END
 };
 
-static const JSConstDoubleSpec AtomicsConstants[] = {
-    {"OK",       AtomicsObject::FutexOK},
-    {"TIMEDOUT", AtomicsObject::FutexTimedout},
-    {"NOTEQUAL", AtomicsObject::FutexNotequal},
-    {0,          0}
-};
-
 JSObject*
 AtomicsObject::initClass(JSContext* cx, Handle<GlobalObject*> global)
 {
     // Create Atomics Object.
     RootedObject objProto(cx, global->getOrCreateObjectPrototype(cx));
     if (!objProto)
         return nullptr;
     RootedObject Atomics(cx, NewObjectWithGivenProto(cx, &AtomicsObject::class_, objProto,
                                                      SingletonObject));
     if (!Atomics)
         return nullptr;
 
     if (!JS_DefineFunctions(cx, Atomics, AtomicsMethods))
         return nullptr;
-    if (!JS_DefineConstDoubles(cx, Atomics, AtomicsConstants))
-        return nullptr;
 
     RootedValue AtomicsValue(cx, ObjectValue(*Atomics));
 
     // Everything is set up, install Atomics on the global object.
     if (!DefineProperty(cx, global, cx->names().Atomics, AtomicsValue, nullptr, nullptr,
                         JSPROP_RESOLVING))
     {
         return nullptr;
--- a/js/src/builtin/AtomicsObject.h
+++ b/js/src/builtin/AtomicsObject.h
@@ -13,23 +13,16 @@
 namespace js {
 
 class AtomicsObject : public JSObject
 {
   public:
     static const Class class_;
     static JSObject* initClass(JSContext* cx, Handle<GlobalObject*> global);
     static bool toString(JSContext* cx, unsigned int argc, Value* vp);
-
-    // Result codes for Atomics.wait().
-    enum FutexWaitResult : int32_t {
-        FutexOK = 0,
-        FutexNotequal = -1,
-        FutexTimedout = -2
-    };
 };
 
 bool atomics_compareExchange(JSContext* cx, unsigned argc, Value* vp);
 bool atomics_exchange(JSContext* cx, unsigned argc, Value* vp);
 bool atomics_load(JSContext* cx, unsigned argc, Value* vp);
 bool atomics_store(JSContext* cx, unsigned argc, Value* vp);
 bool atomics_add(JSContext* cx, unsigned argc, Value* vp);
 bool atomics_sub(JSContext* cx, unsigned argc, Value* vp);
@@ -63,27 +56,33 @@ public:
     void destroyInstance();
 
     // Parameters to wake().
     enum WakeReason {
         WakeExplicit,           // Being asked to wake up by another thread
         WakeForJSInterrupt      // Interrupt requested
     };
 
+    // Result code from wait().
+    enum WaitResult {
+        FutexOK,
+        FutexTimedOut
+    };
+
     // Block the calling thread and wait.
     //
     // The futex lock must be held around this call.
     //
     // The timeout is the number of milliseconds, with fractional
     // times allowed; specify positive infinity for an indefinite wait.
     //
     // wait() will not wake up spuriously.  It will return true and
     // set *result to a return code appropriate for
     // Atomics.wait() on success, and return false on error.
-    bool wait(JSContext* cx, double timeout, AtomicsObject::FutexWaitResult* result);
+    bool wait(JSContext* cx, double timeout, WaitResult* result);
 
     // Wake the thread represented by this Runtime.
     //
     // The futex lock must be held around this call.  (The sleeping
     // thread will not wake up until the caller of Atomics.wake()
     // releases the lock.)
     //
     // If the thread is not waiting then this method does nothing.
--- a/js/src/tests/shell/futex.js
+++ b/js/src/tests/shell/futex.js
@@ -64,16 +64,28 @@ assertThrowsInstanceOf(() => setSharedAr
 if (helperThreadCount() === 0) {
   // Abort if there is no helper thread.
   reportCompare(true,true);
   quit();
 }
 
 ////////////////////////////////////////////////////////////
 
+// wait() returns "not-equal" if the value is not the expected one.
+
+mem[0] = 42;
+
+assertEq(Atomics.wait(mem, 0, 33), "not-equal");
+
+// wait() returns "timed-out" if it times out
+
+assertEq(Atomics.wait(mem, 0, 42, 100), "timed-out");
+
+////////////////////////////////////////////////////////////
+
 // Main is sharing the buffer with the worker; the worker is clearing
 // the buffer.
 
 mem[0] = 42;
 mem[1] = 37;
 mem[2] = DEBUG;
 
 setSharedArrayBuffer(mem.buffer);
@@ -89,17 +101,17 @@ mem[1] = 1337;
 dprint("Sleeping for 2 seconds");
 sleep(2);
 dprint("Waking the main thread now");
 setSharedArrayBuffer(null);
 assertEq(Atomics.wake(mem, 0, 1), 1); // Can fail spuriously but very unlikely
 `);
 
 var then = Date.now();
-assertEq(Atomics.wait(mem, 0, 42), Atomics.OK);
+assertEq(Atomics.wait(mem, 0, 42), "ok");
 dprint("Woke up as I should have in " + (Date.now() - then)/1000 + "s");
 assertEq(mem[1], 1337); // what was written in the worker is read in the main thread
 assertEq(getSharedArrayBuffer(), null); // The worker's clearing of the mbx is visible
 
 ////////////////////////////////////////////////////////////
 
 // Test the default argument to atomics.wake()
 
@@ -108,33 +120,33 @@ setSharedArrayBuffer(mem.buffer);
 evalInWorker(`
 var mem = new Int32Array(getSharedArrayBuffer());
 sleep(2);				// Probably long enough to avoid a spurious error next
 assertEq(Atomics.wake(mem, 0), 1);	// Last argument to wake should default to +Infinity
 `);
 
 var then = Date.now();
 dprint("Main thread waiting on wakeup (2s)");
-assertEq(Atomics.wait(mem, 0, 42), Atomics.OK);
+assertEq(Atomics.wait(mem, 0, 42), "ok");
 dprint("Woke up as I should have in " + (Date.now() - then)/1000 + "s");
 
 ////////////////////////////////////////////////////////////
 
 // A tricky case: while in the wait there will be an interrupt, and in
 // the interrupt handler we will execute a wait.  This is
 // explicitly prohibited (for now), so there should be a catchable exception.
 
 timeout(2, function () {
     dprint("In the interrupt, starting inner wait with timeout 2s");
     Atomics.wait(mem, 0, 42); // Should throw and propagate all the way out
 });
 var exn = false;
 try {
     dprint("Starting outer wait");
-    assertEq(Atomics.wait(mem, 0, 42, 5000), Atomics.OK);
+    assertEq(Atomics.wait(mem, 0, 42, 5000), "ok");
 }
 catch (e) {
     dprint("Got the timeout exception!");
     exn = true;
 }
 finally {
     timeout(-1);
 }
--- a/js/src/vm/CommonPropertyNames.h
+++ b/js/src/vm/CommonPropertyNames.h
@@ -107,16 +107,19 @@
     macro(float64, float64, "float64") \
     macro(Float64x2, Float64x2, "Float64x2") \
     macro(forceInterpreter, forceInterpreter, "forceInterpreter") \
     macro(forEach, forEach, "forEach") \
     macro(format, format, "format") \
     macro(formatToParts, formatToParts, "formatToParts") \
     macro(frame, frame, "frame") \
     macro(from, from, "from") \
+    macro(futexOK, futexOK, "ok") \
+    macro(futexNotEqual, futexNotEqual, "not-equal") \
+    macro(futexTimedOut, futexTimedOut, "timed-out") \
     macro(gcCycleNumber, gcCycleNumber, "gcCycleNumber") \
     macro(GeneratorFunction, GeneratorFunction, "GeneratorFunction") \
     macro(get, get, "get") \
     macro(getInternals, getInternals, "getInternals") \
     macro(getOwnPropertyDescriptor, getOwnPropertyDescriptor, "getOwnPropertyDescriptor") \
     macro(getOwnPropertyNames, getOwnPropertyNames, "getOwnPropertyNames") \
     macro(getPropertyDescriptor, getPropertyDescriptor, "getPropertyDescriptor") \
     macro(global, global, "global") \