Optimize anon content in BBP

RESOLVED FIXED

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

(Blocks: 1 bug)

12 Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
Created attachment 595898 [details] [diff] [review]
wip
Blocks: 716598
OS: Linux → All
Hardware: x86_64 → All
(Assignee)

Comment 1

5 years ago
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)
Attachment #595898 - Flags: review?(jst)
Attachment #595898 - Flags: feedback?(continuation)
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.
Attachment #595898 - Flags: feedback?(continuation) → feedback+
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.
Attachment #595898 - Flags: review?(jst) → review+
(Assignee)

Comment 4

5 years ago
Created attachment 596124 [details] [diff] [review]
patch
(Assignee)

Comment 5

5 years ago
https://hg.mozilla.org/mozilla-central/rev/b3073c517d21
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.