761334 - Trace Malloc Allocs and MaxHeap regressions from bug 752676 (Add pdf.js as an internal handler)

Status: RESOLVED WONTFIX

(Firefox :: PDF Viewer, defect)

Description

[Matt Brubeck (:mbrubeck)]

Bug 752676 apparently caused a regression in the Trace Malloc benchmarks on both Windows and Linux:

http://graphs.mozilla.org/graph.html#tests=[[29,63,18]]&sel=1338818805921.5896,1338829385513.4265&displayrange=7&datatype=running

https://groups.google.com/d/msg/mozilla.dev.tree-management/sN60GtP4aBc/WxksjMpaAroJ

https://wiki.mozilla.org/Performance:Leak_Tools#Trace-malloc

Comment 2

[Brendan Dahl [:bdahl]]

Testing this out locally using leaktest.py on osx I get numbers all over of the place for the max heap with RSD ~2%.  Some of the smallest max heap sizes have been with the above changeset applied.  However, on average I see the max heap grow ~100KB with the pdf.js changeset.

I expect the max heap to get somewhat bigger since we're now adding two new components PdfJs.jsm and the PdfStreamConverter.js.  Also, on firstrun pdf.js has to go in and take over as the default pdf reader.

One thing we could do to lessen the memory is instead of programatically registering the stream converter we could put it back in the chrome.manifest so PdfStreamConverter isn't loaded until the first pdf is loaded.  This though would mean the stream converter is on by default which has pros/cons.

Comment 3

[Alex Keybl [:akeybl]]

It's not clear to me what the user impact would be here (seems like a leak regression), or how commonly it would occur. Tracking nonetheless to make sure this is followed up on.

Comment 4

[Alex Keybl [:akeybl]]

Given bug 773397, tracking for FF16's release but not FF15. Would still love to get clarification on the user impact here.

Comment 5

[Brendan Dahl [:bdahl]]

Alex: I must have missed your previous comment in my bugmail.  The regression doesn't seem to be a "leak".  It's just a regression in the maximum amount of memory firefox uses(~2MB more) during that test, which is then cleaned up.  I still haven't heard anything definitive if this is too much for what is happening on first run or not.

Comment 6

[Alex Keybl [:akeybl]]

(In reply to Brendan Dahl from comment #5)
> Alex: I must have missed your previous comment in my bugmail.  The
> regression doesn't seem to be a "leak".  It's just a regression in the
> maximum amount of memory firefox uses(~2MB more) during that test, which is
> then cleaned up.  I still haven't heard anything definitive if this is too
> much for what is happening on first run or not.

OK - I've added [Snappy] to the whiteboard to get their expertise on whether or not this test is actually concerning. Also untracking for FF16 since we won't be ready to ship till FF17 at least.

Comment 7

[(dormant account)]

This might be a memshink bug, but there is nothing here for snappy.

Comment 8

[Nicholas Nethercote [inactive]]

18 months have passed. I don't think this bug is worth keeping open.