clean up cycle collector JS tracing callbacks

RESOLVED FIXED in mozilla16

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

Trunk
mozilla16
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(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) {
    NS_IMPL_CYCLE_COLLECTION_TRACE_JS_CALLBACK(tmp->mResultArrayBuffer,
                                               "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);
    NS_IMPL_CYCLE_COLLECTION_TRACE_JS_CALLBACK(gcThing, "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.
Posted patch clean it up! (obsolete) — Splinter Review
Assignee: nobody → continuation
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!

https://hg.mozilla.org/integration/mozilla-inbound/rev/b7d5ee4528c8
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/b7d5ee4528c8
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.