Bug 1145201: Remove unnecessary SpiderMonkey internal job queue reset. r=jorendorff
authorJim Blandy <jimb@mozilla.com>
Thu, 03 Jan 2019 15:19:09 +0000
changeset 452450 ba5d3ec164f2670f64eee231e55926c37cb9a3c2
parent 452449 df23b1fa16f500545bb1c319b078788f842a94fa
child 452451 4a8c79b919b2b565219da4adf9c7f74686d4296b
push id75444
push userjblandy@mozilla.com
push dateThu, 03 Jan 2019 19:22:43 +0000
treeherderautoland@ba5d3ec164f2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1145201
milestone66.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 1145201: Remove unnecessary SpiderMonkey internal job queue reset. r=jorendorff The call to `cx->jobQueue->reset()` doesn't do anything that isn't also accomplished by the call to `fop->delete_(cx->jobQueue.ref())` two lines later. Since `cx->jobQueue` is a `ThreadData<PersistentRooted<JobQueue>*>`, the `PersistentRooted` actually owns the `JobQueue` (despite the field's name `ptr`), rather than holding a pointer to it, so deleting the `PersistentRooted` invokes the `JobQueue` destructor. In more detail: `JSContext::jobQueue` is a `ThreadData<PersistentRooted<JobQueue>*>`, so the call `cx->jobQueue->reset()` performs the following steps: - Call `ProtectedData::operator->`, obtaining a (const reference to a non-const) pointer `PersistentRooted<JobQueue>*`. - Call `PersistentRooted::reset`, which move-assigns a fresh `JobQueue` to the `ptr` member. Note that `PersistentRooted<JobQueue>::ptr` is a `JobQueue`, *not* a pointer. But the subsequent call to `fop->delete_(cx->jobQueue.ref())` will perform the following steps: - Call `ProtectedData::ref`, obtaining a (reference to a) `PersistentRooted<JobQueue>*` pointer. - Call `PersistentRooted<JobQueue>`'s destructor, which destructs `ptr`. Since `ptr` is a `JobQueue`, this calls the `JobQueue`'s destructor, safely freeing its resources. Differential Revision: https://phabricator.services.mozilla.com/D15120
js/src/vm/JSContext.cpp
--- a/js/src/vm/JSContext.cpp
+++ b/js/src/vm/JSContext.cpp
@@ -172,17 +172,16 @@ JSContext* js::NewContext(uint32_t maxBy
   return cx;
 }
 
 static void FreeJobQueueHandling(JSContext* cx) {
   if (!cx->jobQueue) {
     return;
   }
 
-  cx->jobQueue->reset();
   FreeOp* fop = cx->defaultFreeOp();
   fop->delete_(cx->jobQueue.ref());
   cx->getIncumbentGlobalCallback = nullptr;
   cx->enqueuePromiseJobCallback = nullptr;
   cx->enqueuePromiseJobCallbackData = nullptr;
 }
 
 void js::DestroyContext(JSContext* cx) {