Last Comment Bug 754299 - Update gcMaxMallocBytes of existing compartments in JSRuntime::setGCMaxMallocBytes
: Update gcMaxMallocBytes of existing compartments in JSRuntime::setGCMaxMalloc...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla15
Assigned To: Till Schneidereit [:till]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-11 08:24 PDT by Till Schneidereit [:till]
Modified: 2012-05-16 03:32 PDT (History)
3 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Updates existing compartments in JSRuntime::setGCMaxMallocBytes (1.80 KB, patch)
2012-05-11 08:24 PDT, Till Schneidereit [:till]
wmccloskey: review+
Details | Diff | Review
Updates existing compartments in JSRuntime::setGCMaxMallocBytes, review comments addressed, carrying r+ (1.79 KB, patch)
2012-05-15 06:14 PDT, Till Schneidereit [:till]
till: review+
Details | Diff | Review

Description Till Schneidereit [:till] 2012-05-11 08:24:58 PDT
Created attachment 623147 [details] [diff] [review]
Updates existing compartments in JSRuntime::setGCMaxMallocBytes

+++ 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.
Comment 1 Bill McCloskey (:billm) 2012-05-14 11:46:11 PDT
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(); c.next())

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.
Comment 2 Till Schneidereit [:till] 2012-05-15 06:14:26 PDT
Created attachment 624012 [details] [diff] [review]
Updates existing compartments in JSRuntime::setGCMaxMallocBytes, review comments addressed, carrying r+

(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 ;) )
Comment 3 Ryan VanderMeulen [:RyanVM] 2012-05-15 15:34:52 PDT
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!

https://hg.mozilla.org/integration/mozilla-inbound/rev/cf5a9fb656c4
Comment 4 Till Schneidereit [:till] 2012-05-15 17:00:56 PDT
Thanks for the checkin, Ryan - and sorry for leaving out the commit message. I normally do include them but forgot that for this one.
Comment 5 Ed Morley [:emorley] 2012-05-16 03:32:54 PDT
https://hg.mozilla.org/mozilla-central/rev/cf5a9fb656c4

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