789667 - Profiler creates JS object containing profile data before dumping to disk

Status: RESOLVED WORKSFORME

(Core :: Gecko Profiler, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

dbaron and I are seeing b2g crash while trying to dump profile data on otoro.  This worked 2 days ago (Sept 5).  The crashes look like OOM; nothing in logcat.

In the meantime, http://hg.mozilla.org/mozilla-central/rev/5f8055d8dde1 looks quite suspicious.

A million sample entries isn't all that big of a deal, except for the code that dumps the profile

        JSAutoCompartment autoComp(cx, obj);
        JSObject* profileObj = mozilla_sampler_get_profile_data(cx);
        jsval val = OBJECT_TO_JSVAL(profileObj);
        JS_Stringify(cx, &val, nullptr, JSVAL_NULL, WriteCallback, &stream);
        stream.close();

If I'm reading things correctly, we create a temporary JS object out of all the profile samples, and then serialize the object to JSON.  With a million samples and not much memory, that hurts! :)

For now I'm going to knock the default sample size down for memory-constrained platforms, but we should serialize more efficiently in this common case, and leave the JS object path for uncommon consumers.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Stanch the bleeding

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Bug 789717 also helps with this problem.

Comment 3

[Mike Hommey [:glandium]]

I wonder if it wouldn't make sense to create the json string directly.

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Or use typed arrays for an efficient binary transfer format.

Comment 5

[Benoit Girard (:BenWa)]

What we discussed is we need to way to get the profile in pieces assuming we can't fit it into the working set. Without this assumption we need at least the memory of the profile bufffer free when dumping the profile but in practice we will need much memory more since entries in the circular buffer point to string in the text section.

Comment 6

[Benoit Girard (:BenWa)]

Since we can now save the profile directly to a json string without using the JSEngine. This was added in bug 799640. We use this to save a profile at the end of XRE_main after the JS engine is off. I think the original issue is fixed but we don't do what is suggested in Comment 4. That should get it's own new bug.