Update gcMaxMallocBytes of existing compartments in JSRuntime::setGCMaxMallocBytes

RESOLVED FIXED in mozilla15

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: till, Assigned: till)

Tracking

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

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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.
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(); 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.
Attachment #623147 - Flags: review?(wmccloskey) → review+
(Assignee)

Comment 2

5 years ago
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 ;) )
Assignee: general → tschneidereit+bmo
Attachment #623147 - Attachment is obsolete: true
Attachment #624012 - Flags: review+
(Assignee)

Updated

5 years ago
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!

https://hg.mozilla.org/integration/mozilla-inbound/rev/cf5a9fb656c4
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → mozilla15
(Assignee)

Comment 4

5 years ago
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

5 years ago
https://hg.mozilla.org/mozilla-central/rev/cf5a9fb656c4
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.