Closed
Bug 755992
Opened 12 years ago
Closed 12 years ago
Reduce about:memory perturbation some more
Categories
(Toolkit :: about:memory, defect)
Toolkit
about:memory
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: nmaier, Assigned: nmaier)
Details
(Whiteboard: [MemShrink])
Attachments
(2 files, 2 obsolete files)
3.81 KB,
patch
|
n.nethercote
:
review+
|
Details | Diff | Splinter Review |
1.21 KB,
patch
|
n.nethercote
:
review+
|
Details | Diff | Splinter Review |
Continue what bug 722972 started. I got a few patches that will improve the memory consumption of about:memory and hence reduce the perturbation caused by having to generate the page.
Comment 1•12 years ago
|
||
We might be butting heads between here and bug 755583... I've already incorporated some of the suggestions you sent me via email, I'll be posting them to bug 755583 later today.
Assignee | ||
Comment 2•12 years ago
|
||
(In reply to Nicholas Nethercote [:njn] from comment #1) > We might be butting heads between here and bug 755583... I've already > incorporated some of the suggestions you sent me via email, I'll be posting > them to bug 755583 later today. OK. I thought of bug 755583 as a more "overall" thing, instead of smallish but viable optimization I wanted to propose here. Got five patches... Want me to attach to this bug or kill it and move over to bug 755583?
Comment 3•12 years ago
|
||
Eh, might as well do it here, to stop the other bug having too many patches.
Assignee | ||
Comment 4•12 years ago
|
||
String.match() create match objects, which in turn will copy the matched part. RegExp.test() does avoid that, as it returns a primitive boolean.
Attachment #624650 -
Flags: review?(n.nethercote)
Assignee | ||
Comment 5•12 years ago
|
||
Element.createTextNode() does create a new Node object in js-land. Using .textNode directly, that overhead can be avoided.
Attachment #624652 -
Flags: review?(n.nethercote)
Assignee | ||
Comment 6•12 years ago
|
||
appendTextNode() uses .createTextNode(), which incurs a lot of memory overhead. In the code appendTextNode() is often used to add some newlines for copy/paste formatting. Some of these newlines can easily be collapsed into existing nodes without creating new ones.
Attachment #624655 -
Flags: review?(n.nethercote)
Comment 7•12 years ago
|
||
Comment on attachment 624650 [details] [diff] [review] Part 1: Use RegExp.test(String) Review of attachment 624650 [details] [diff] [review]: ----------------------------------------------------------------- Heh, I have the same patch in bug 755583, I already r+'d it because you wrote it. Let's keep it over there.
Attachment #624650 -
Flags: review?(n.nethercote)
Assignee | ||
Comment 8•12 years ago
|
||
Allocating gcthings, such as Objects, always incurs a slight memory penalty. Avoid the extraIndentArray when not needed. This is one of the top spots of object allocations.
Attachment #624656 -
Flags: review?(n.nethercote)
Comment 9•12 years ago
|
||
Comment on attachment 624652 [details] [diff] [review] Part 2: Avoid Node.createTextNode() in appendElementWithText() Review of attachment 624652 [details] [diff] [review]: ----------------------------------------------------------------- Ditto, except I added a comment :)
Attachment #624652 -
Flags: review?(n.nethercote)
Comment 10•12 years ago
|
||
Comment on attachment 624655 [details] [diff] [review] Part 1: Avoid some appendTextNode() calls Review of attachment 624655 [details] [diff] [review]: ----------------------------------------------------------------- All of these occur in places that are executed infrequently, so they won't affect performance at all. But I'll r+ because it makes the code shorter. Do you want me to land your patches, since I'll be doing the ones in bug 755583? I'm happy to do so.
Attachment #624655 -
Flags: review?(n.nethercote) → review+
Assignee | ||
Comment 11•12 years ago
|
||
(In reply to Nicholas Nethercote [:njn] from comment #10) > Do you want me to land your patches, since I'll be doing the ones in bug > 755583? I'm happy to do so. Sure. I don't have any commit access myself, anyway.
Comment 12•12 years ago
|
||
Comment on attachment 624656 [details] [diff] [review] Part 2: Avoid creating extraIndentArray when not necessary Review of attachment 624656 [details] [diff] [review]: ----------------------------------------------------------------- About a 3--4% improvement in the cumulative-GC-bytes measurement.
Attachment #624656 -
Flags: review?(n.nethercote) → review+
Comment 13•12 years ago
|
||
will this help with the long time it took to me to run about:memory for bug 756261? I clicked on unresponsive script at least 5 times.
Updated•12 years ago
|
Attachment #624650 -
Attachment is obsolete: true
Updated•12 years ago
|
Attachment #624652 -
Attachment is obsolete: true
Updated•12 years ago
|
Attachment #624655 -
Attachment description: Part 3: Avoid some appendTextNode() calls → Part 1: Avoid some appendTextNode() calls
Updated•12 years ago
|
Attachment #624656 -
Attachment description: Part 4: Avoid creating extraIndentArray when not necessary → Part 2: Avoid creating extraIndentArray when not necessary
Comment 14•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/6905cb15e8dd https://hg.mozilla.org/integration/mozilla-inbound/rev/276f4c406f07
Target Milestone: --- → mozilla15
Comment 15•12 years ago
|
||
(In reply to Wayne Mery (:wsmwk) from comment #13) > will this help with the long time it took to me to run about:memory for bug > 756261? > I clicked on unresponsive script at least 5 times. Not sure. But in that case, about:memory's slowness is a consequence of the memory consumption being so high -- it iterates over every object in the JS heap, and if your JS heap is multiple GB that can be very slow. The real question is why is your JS heap so large, and I discussed that in 756261 comment 4.
Comment 16•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6905cb15e8dd https://hg.mozilla.org/mozilla-central/rev/276f4c406f07
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•