The default bug view has changed. See this FAQ.

Reduce about:memory perturbation some more

RESOLVED FIXED in mozilla15

Status

()

Toolkit
about:memory
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: nmaier, Assigned: nmaier)

Tracking

unspecified
mozilla15
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink])

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

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

5 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?
Eh, might as well do it here, to stop the other bug having too many patches.
(Assignee)

Comment 4

5 years ago
Created attachment 624650 [details] [diff] [review]
Part 1: Use RegExp.test(String)

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

5 years ago
Created attachment 624652 [details] [diff] [review]
Part 2: Avoid Node.createTextNode() in appendElementWithText()

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

5 years ago
Created attachment 624655 [details] [diff] [review]
Part 1: Avoid some appendTextNode() calls

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 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

5 years ago
Created attachment 624656 [details] [diff] [review]
Part 2: Avoid creating extraIndentArray when not necessary

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 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 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

5 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 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+
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.
Attachment #624650 - Attachment is obsolete: true
Attachment #624652 - Attachment is obsolete: true
Attachment #624655 - Attachment description: Part 3: Avoid some appendTextNode() calls → Part 1: Avoid some appendTextNode() calls
Attachment #624656 - Attachment description: Part 4: Avoid creating extraIndentArray when not necessary → Part 2: Avoid creating extraIndentArray when not necessary
https://hg.mozilla.org/integration/mozilla-inbound/rev/6905cb15e8dd
https://hg.mozilla.org/integration/mozilla-inbound/rev/276f4c406f07
Target Milestone: --- → mozilla15
(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.
https://hg.mozilla.org/mozilla-central/rev/6905cb15e8dd
https://hg.mozilla.org/mozilla-central/rev/276f4c406f07
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.