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 291449 fd3ca174abe074fbde07dd3fb7efe1d5f5da477a
parent 291448 63bdfecc99f488142d1601f381f6241fd22ddb92
child 291450 823040191adc78b25f1c8ec6d0523c209b97598f
push id74572
push userlhansen@mozilla.com
push dateSun, 03 Apr 2016 13:49:04 +0000
treeherdermozilla-inbound@fd3ca174abe0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjolesen
bugs1260835
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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") \