clean up cycle collector JS tracing callbacks

RESOLVED FIXED in mozilla16



7 years ago
7 years ago


(Reporter: mccr8, Assigned: mccr8)


Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)



(1 attachment, 1 obsolete attachment)

I noticed that there are a few ways the JS tracing callbacks could be cleaned up while looking at bug 754151.

A common pattern is:
  if(tmp->mResultArrayBuffer) {
This has two issues.  First, TRACE_JS_CALLBACK already does a nullcheck, so we don't need to do another one.  Secondly, the inner part is the same as NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK, so we might as well use that.

Another common pattern that is used all over the place is:
  if (JSVAL_IS_GCTHING(tmp->mCachedKey)) {
    void *gcThing = JSVAL_TO_GCTHING(tmp->mCachedKey);

First of all, we can replace this with a macro NS_IMPL_CYCLE_COLLECTION_TRACE_JSVAL_MEMBER_CALLBACK(mCachedKey), because it is a very common pattern.  The second improvement we can make is to replace JSVAL_IS_GCTHING with JSVAL_IS_TRACEABLE.  Traceable just means non-null GCthing.  This allows us to eliminate the inner null check.  Though it involves an or-branch, so maybe it isn't actually any better.
Created attachment 623342 [details] [diff] [review]
clean it up!
Assignee: nobody → continuation
Created attachment 624084 [details] [diff] [review]
clean it up (removed the fairly pointless comment)

Try run looked fine here.  Ben, I'm flagging you for review because most of these changes are in IndexedDB and you know the CC.
Attachment #623342 - Attachment is obsolete: true
Attachment #624084 - Flags: review?(bent.mozilla)
Comment on attachment 624084 [details] [diff] [review]
clean it up (removed the fairly pointless comment)

Review of attachment 624084 [details] [diff] [review]:

Sorry this waited so long :(
Attachment #624084 - Flags: review?(bent.mozilla) → review+
(In reply to ben turner [:bent] from comment #3)
> Sorry this waited so long :(

No problem, this isn't anything time critical. Just part of the slow fight against entropy. Thanks for the review!
Target Milestone: --- → mozilla16
Last Resolved: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.