Closed Bug 754299 Opened 10 years ago Closed 10 years ago

Update gcMaxMallocBytes of existing compartments in JSRuntime::setGCMaxMallocBytes


(Core :: JavaScript Engine, defect)

Not set





(Reporter: till, Assigned: till)



(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #723350 +++

As billm noted in bug 723350 comment 19, existing compartments don't get updated when the value is changed in JSRuntime.

The attached patch moves the implementation of JSRuntime::setGCMaxMallocBytes to jscntxt.cpp and adds an iteration over all existing compartments invoking JSCompartment::setGCMaxMallocBytes with the new value.
Attachment #623147 - Flags: review?(wmccloskey)
Comment on attachment 623147 [details] [diff] [review]
Updates existing compartments in JSRuntime::setGCMaxMallocBytes

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

::: js/src/jscntxt.cpp
@@ +1102,5 @@
>  void
> +JSRuntime::setGCMaxMallocBytes(size_t value)
> +{
> +   gcMaxMallocBytes = value;
> +   for (js::gc::GCCompartmentsIter c(this); !c.done();

Please change js::gc::GCCompartmentsIter to CompartmentsIter. You don't need the namespaces because of the using declarations higher in the file. And you want CompartmentsIter instead of GCCompartmentsIter. GCCompartmentsIter iterates over the compartments that are being collected during a GC (and asserts if no GC is running), while CompartmentsIter iterates over all compartments.
Attachment #623147 - Flags: review?(wmccloskey) → review+
(In reply to Bill McCloskey (:billm) from comment #1)
> Please change js::gc::GCCompartmentsIter to CompartmentsIter.

Thanks for the review! (And for being gentle about that blunder ;) )
Assignee: general → tschneidereit+bmo
Attachment #623147 - Attachment is obsolete: true
Attachment #624012 - Flags: review+
Keywords: checkin-needed
Thanks for the patch, Till! Please be sure to add a commit message to the patches you post to make life easier for checking in. Thanks!
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → mozilla15
Thanks for the checkin, Ryan - and sorry for leaving out the commit message. I normally do include them but forgot that for this one.
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.