754299 - Update gcMaxMallocBytes of existing compartments in JSRuntime::setGCMaxMallocBytes

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Till Schneidereit [:till]]

Attachment: 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 [inactive unless it's an emergency] (:billm)]

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]]

Attachment: 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]]

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]]

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]]

https://hg.mozilla.org/mozilla-central/rev/cf5a9fb656c4