1227914 - Limit memory usage of TraceLogger ContinuousSpace

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Hannes Verschore [:h4writer]]

Currently when there is not enough space in the ContinuousSpace, we will just double the about the memory used. Upon fail, we write/drop the data and reuse the memory we already had. Now it might be good to have some upper-limit that the ContinuousSpace can use. Just for safety.

Comment 1

[Hannes Verschore [:h4writer]]

Attachment: Patch

Put an upperbound on how big ContinuousSpace can get. Went for 200MB. Just to make sure we don't take all memory.

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8691922 [details] [diff] [review]
Patch

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

r=me if you're fine with the change below, which makes things easier to understand imo.

::: js/src/vm/TraceLoggingTypes.h
@@ +204,5 @@
> +        if (nCapacity * sizeof(T) > CONTINUOUSSPACE_LIMIT) {
> +            if ((size_ + count) * sizeof(T) > CONTINUOUSSPACE_LIMIT)
> +                return false;
> +            nCapacity = size_ + count;
> +        }

It took me some time to realize why this is correct. Instead, can you do:

uint32_t nCapacity = capacity_ * 2;
if (size_ + count > nCapacity || nCapacity * sizeof(T) > CONTINUOUSSPACE_LIMIT)
    nCapacity = size_ + count;
if (nCapacity * sizeof(T) > CONTINUOUSSPACE_LIMIT)
    return false;

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Or even

uint32_t nCapacity = capacity_ * 2;
if (size_ + count > nCapacity || nCapacity * sizeof(T) > CONTINUOUSSPACE_LIMIT) {
    nCapacity = size_ + count;
    if (nCapacity * sizeof(T) > CONTINUOUSSPACE_LIMIT)
        return false;
}

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/541a97a551cb

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/541a97a551cb