1294404 - Merge PerThreadDataFriendFields and ContextFriendFields

Status: RESOLVED FIXED

Description

[Jan de Mooij [:jandem]]

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(-)

Comment 1

[Jan de Mooij [:jandem]]

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

[Terrence Cole [:terrence]]

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!

Comment 3

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Comment on attachment 8780055 [details] [diff] [review]
Patch

r=me on the DOM bits.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e0d9eb7eb05
Merge PerThreadDataFriendFields and ContextFriendFields, clean up APIs. r=terrence,bz

Comment 5

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/7e0d9eb7eb05