Last Comment Bug 716042 - GC: missing barriers in TradeGuts
: GC: missing barriers in TradeGuts
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla13
Assigned To: Terrence Cole [:terrence]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: 673454
  Show dependency treegraph
 
Reported: 2012-01-06 13:21 PST by Terrence Cole [:terrence]
Modified: 2012-02-03 11:04 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (2.49 KB, patch)
2012-01-06 14:02 PST, Terrence Cole [:terrence]
wmccloskey: review+
Details | Diff | Splinter Review

Description User image Terrence Cole [:terrence] 2012-01-06 13:21:27 PST
Trigger post barriers when we TradeGuts.  Pre barriers are taken care of already by manually marking.  We need similar treatment for post barriers, taking care to handle fixed slots correctly.
Comment 1 User image Terrence Cole [:terrence] 2012-01-06 14:02:35 PST
Created attachment 586561 [details] [diff] [review]
v1
Comment 2 User image Bill McCloskey (:billm) 2012-02-01 17:00:25 PST
Comment on attachment 586561 [details] [diff] [review]
v1

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

::: js/src/jsobj.cpp
@@ +3578,5 @@
> +         * below, in common with the other case.
> +         */
> +        for (size_t i = 0; i < a->numFixedSlots(); ++i) {
> +            Value *slotA = const_cast<Value *>(a->getFixedSlot(i));
> +            Value *slotB = const_cast<Value *>(b->getFixedSlot(i));

I think this would look better if you did:
    HeapValue *slotA = &a->getFixedSlotRef(i);

@@ +3642,5 @@
> +#ifdef JSGC_GENERATIONAL
> +    Shape::writeBarrierPost(a->shape_, (void *)&a->shape_);
> +    Shape::writeBarrierPost(b->shape_, (void *)&b->shape_);
> +    types::TypeObject::writeBarrierPost(a->type_, (void *)&a->type_);
> +    types::TypeObject::writeBarrierPost(b->type_, (void *)&b->type_);

I don't think you need these (void *) casts. I think they're used in gc/Barrier.h because some of the stuff is const.
Comment 3 User image Bill McCloskey (:billm) 2012-02-01 17:03:58 PST
Also, please don't put two spaces after the period in the comment. See the SpiderMonkey style guide.
Comment 4 User image Terrence Cole [:terrence] 2012-02-02 16:43:01 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3a0eab64d04
Comment 5 User image Ed Morley [:emorley] 2012-02-03 11:04:40 PST
https://hg.mozilla.org/mozilla-central/rev/e3a0eab64d04

Note You need to log in before you can comment on or make changes to this bug.