Status
()
People
(Reporter: jandem, Assigned: jandem)
Tracking
Firefox Tracking Flags
(firefox51 fixed)
Details
Attachments
(1 attachment)
61.75 KB,
patch
|
terrence
:
review+
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
Created attachment 8780055 [details] [diff] [review] Patch A lot of goodness comes from this: * Moving the root lists and stack limits to ContextFriendFields makes accessing them more efficient (it eliminates the cx->runtime_ dereference). * It simplifies the rooting APIs: it's now only possible to construct Rooted<> and PersistentRooted<> from the {Rooting,Exclusive,JS}Context classes. It's no longer possible to use PerThreadData or JSRuntime for this. * It lets us remove PerThreadDataFriendFields, the js::GetRuntime friend API, etc. * 217 insertions(+), 254 deletions(-)
Attachment #8780055 -
Flags: review?(terrence)
Attachment #8780055 -
Flags: review?(bzbarsky)
(Assignee) | ||
Comment 1•3 years ago
|
||
bz, the changes to the dom/ code are just to get rid of the js::GetRuntime call hidden there. I replaced the JSRuntime* with a JS::RootingContext* (and a debug-only mCalledInit bit).
Comment 2•3 years ago
|
||
Comment on attachment 8780055 [details] [diff] [review] Patch Review of attachment 8780055 [details] [diff] [review]: ----------------------------------------------------------------- This is great! ::: js/public/RootingAPI.h @@ +666,5 @@ > + inline js::RootedListHeads& rootLists(js::ContextFriendFields* cx) { > + if (JS::Zone* zone = cx->zone_) > + return JS::shadow::Zone::asShadowZone(zone)->stackRoots_; > + MOZ_ASSERT(cx->isJSContext); > + return cx->roots.stackRoots_; Yup! It should be only globals (and maybe some atoms) that get touched when we are outside JSAutoCompartment and these all very much happen on the main thread. @@ +994,4 @@ > } > > + // Disallow ExclusiveContext*. > + js::RootLists& rootLists(js::ContextFriendFields* cx) = delete; \o/ ::: js/src/gc/Zone.cpp @@ +299,5 @@ > Zone::notifyObservingDebuggers() > { > for (CompartmentsInZoneIter comps(this); !comps.done(); comps.next()) { > JSRuntime* rt = runtimeFromAnyThread(); > + RootedGlobalObject global(rt->contextFromMainThread(), comps->unsafeUnbarrieredMaybeGlobal()); It worries me that we used runtimeFromAnyThread in the line above this rather than runtimeFromMainThread. ::: js/src/jspubtd.h @@ -449,5 @@ > - reinterpret_cast<const char*>(rt) + RuntimeMainThreadOffset); > - } > - > - template <typename T> friend class JS::Rooted; > -}; \o/ \o/ \o/ All this complexity when we could have just passed the stack limit around!
Attachment #8780055 -
Flags: review?(terrence) → review+
![]() |
||
Comment 3•3 years ago
|
||
Comment on attachment 8780055 [details] [diff] [review] Patch r=me on the DOM bits.
Attachment #8780055 -
Flags: review?(bzbarsky) → review+
Pushed by jandemooij@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7e0d9eb7eb05 Merge PerThreadDataFriendFields and ContextFriendFields, clean up APIs. r=terrence,bz
Comment 5•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7e0d9eb7eb05
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in
before you can comment on or make changes to this bug.
Description
•