792237 - don't expose outstanding request count, just whether a context is "active"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

Attachment: patch

We currently expose the exact request count of a context when all we need is the boolean query "is anything running in this context".

Comment 1

[Andrew McCreight [:mccr8]]

Comment on attachment 662317 [details] [diff] [review]
patch

Review of attachment 662317 [details] [diff] [review]:
-----------------------------------------------------------------

Nice! I'm glad this weird overprecision will be gone. I don't know if Olli wants to look at this too, as he understands the outstanding request stuff a little better than me, but it looks straightforward enough.

::: dom/base/nsJSEnvironment.cpp
@@ +1207,5 @@
>  nsJSContext::GetCCRefcnt()
>  {
>    nsrefcnt refcnt = mRefCnt.get();
>    if (NS_LIKELY(mContext))
> +    refcnt += js::ContextHasOutstandingRequests(mContext) ? 1 : 0;

This might be better as:

if (mContext && js::ContextHasOutstandingblahblah) {
  refcnt += 1;
}

I doubt this code is perf sensitive enough that the NS_LIKELY is worth making the line longer. You should also add the braces if you are going to touch all of this anyways, to match browser style.

Maybe put a comment here like "See JSContextParticipant::TraverseImpl for explanation."

::: js/xpconnect/src/nsXPConnect.cpp
@@ +923,5 @@
> +        // JSContexts do not have an internal refcount and always have a single
> +        // owner (e.g., nsJSContext). Thus, the default refcount is 1. However,
> +        // in the (abnormal) case of synchronous cycle-collection, the context
> +        // may be actively executing code in which case we want to treat it as
> +        // rooted by adding an extra refcount.

Great comment, thanks.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Read the patch quickly, looks ok to me.

Comment 3

[Luke Wagner [:luke]]

Thanks!

https://hg.mozilla.org/integration/mozilla-inbound/rev/6be84c0e0f0e

Comment 4

[Graeme McCutcheon [:graememcc]]

https://hg.mozilla.org/mozilla-central/rev/6be84c0e0f0e