Last Comment Bug 725867 - Optimize anon content in BBP
: Optimize anon content in BBP
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: 12 Branch
: All All
: -- normal (vote)
: ---
Assigned To: Olli Pettay [:smaug]
:
Mentors:
Depends on:
Blocks: 716598
  Show dependency treegraph
 
Reported: 2012-02-09 16:10 PST by Olli Pettay [:smaug]
Modified: 2012-02-10 12:39 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
wip (1.61 KB, patch)
2012-02-09 16:10 PST, Olli Pettay [:smaug]
jst: review+
continuation: feedback+
Details | Diff | Splinter Review
patch (2.20 KB, patch)
2012-02-10 11:42 PST, Olli Pettay [:smaug]
no flags Details | Diff | Splinter Review

Description Olli Pettay [:smaug] 2012-02-09 16:10:56 PST
Created attachment 595898 [details] [diff] [review]
wip
Comment 1 Olli Pettay [:smaug] 2012-02-10 06:43:46 PST
Comment on attachment 595898 [details] [diff] [review]
wip

This is still reasonable conservative, but helps with XUL based chrome UI
which has plenty of elements in anonymous content.

I'd like to find better way to ensure that anonymous elements in a document
are certainly alive, but haven't found that yet.
(document keeps non-anonymous elements alive)
Comment 2 Andrew McCreight [:mccr8] 2012-02-10 09:24:55 PST
Comment on attachment 595898 [details] [diff] [review]
wip

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

::: content/base/src/nsGenericElement.cpp
@@ +4537,4 @@
>    nsIDocument* currentDoc = aNode->GetCurrentDoc();
>    if (currentDoc &&
> +      nsCCUncollectableMarker::InGeneration(currentDoc->GetMarkedCCGeneration()) &&
> +      // If aNode is not optimizable, but it is still an element

"but it is still" should probably be "but is"

@@ +4542,5 @@
> +      // we can act as if it was optimizable. When the primary frame dies, node
> +      // will end up to the purple buffer because of the refcount change.
> +      (!unoptimizable ||
> +       (currentDoc->GetShell() && aNode->IsElement() &&
> +        aNode->AsElement()->GetPrimaryFrame()))) {

Maybe turn these two lines into a separate function with a descriptive name, then move the comment up there?  I don't understand any of the unoptimizable stuff so I can't really comment on that.
Comment 3 Johnny Stenback (:jst, jst@mozilla.com) 2012-02-10 10:05:48 PST
Comment on attachment 595898 [details] [diff] [review]
wip

+      // If aNode is not optimizable, but it is still an element
+      // with a frame in a document which has currently active presshell,
+      // we can act as if it was optimizable. When the primary frame dies, node

"the node", or "aNode".

+      // will end up to the purple buffer because of the refcount change.
+      (!unoptimizable ||
+       (currentDoc->GetShell() && aNode->IsElement() &&
+        aNode->AsElement()->GetPrimaryFrame()))) {

What Andrew said. Maybe put the code after || in a static inlined helper named NodeHasFrame() or somesuch?

r=jst with that.
Comment 4 Olli Pettay [:smaug] 2012-02-10 11:42:19 PST
Created attachment 596124 [details] [diff] [review]
patch
Comment 5 Olli Pettay [:smaug] 2012-02-10 12:39:27 PST
https://hg.mozilla.org/mozilla-central/rev/b3073c517d21

Note You need to log in before you can comment on or make changes to this bug.