[CC] consider optimizing CC traversal of nsContentSink

RESOLVED WONTFIX

Status

()

RESOLVED WONTFIX
7 years ago
6 years ago

People

(Reporter: mccr8, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
nsContentSink can have hundreds of children that are currently displayed in the document, and can sometimes stay alive for a long time.  See previous discussion in bug 705582.
(Reporter)

Comment 1

7 years ago
The crudest thing we could do would be to not note any children that are currently displayed in the document, by filtering them as we iterate over them during traversal.  It would be better if we could tell immediately if we could skip iterating over the children.

Comment 2

7 years ago
We could check if document (which is in CC generation) owns the parser and content sink.
(does it go something like document->parser->contentsink ?)
If so, the content sink won't be unlinked anyway, and we can skip traversing any of the elements
content sink owns.

Updated

7 years ago
Assignee: nobody → bugs

Comment 3

7 years ago
Grrr, we have nsIContentSink and nsContentSink and they are somewhat unrelated objects.
nsContentSink does not implement nsIContentSink o_O

Updated

7 years ago
Assignee: bugs → nobody

Comment 4

7 years ago
Created attachment 585445 [details] [diff] [review]
WIP

This would work if the class hierarchy was sane.

Henri, can you think of any easy way to compare the identities of objects.
No QIing to nsISupports.
(In reply to Olli Pettay [:smaug] from comment #4)
> Henri, can you think of any easy way to compare the identities of objects.
> No QIing to nsISupports.

We could add 
virtual nsIContentSink* GetAsIContentSink() = 0;
to nsContentSink and implement it as |return this;| in nsHtml5TreeOpExecutor and nsXMLContentSink.

Or we could add a getter for the active parser (nsIParser) to nsIDocument and compare that with the sink's mParser.

In general, the ancient design where code outside the parser gets to hold a reference to nsIContentSink as opposed to just getting to hold a reference to nsIParser is terrible. Now that we don't have artificial module dll boundaries and everything goes into libxul anyway, we should get rid of the dual nsIParser/nsIContentSink design in due course (by implementing my XML code path rewrite plan).
(In reply to Henri Sivonen (:hsivonen) from comment #5)
> We could add 
> virtual nsIContentSink* GetAsIContentSink() = 0;
> to nsContentSink and implement it as |return this;| in nsHtml5TreeOpExecutor
> and nsXMLContentSink.

Let's do this.

> Or we could add a getter for the active parser (nsIParser) to nsIDocument
> and compare that with the sink's mParser.

I'm probably going to make a refactoring that will interfere with this solution.
(Reporter)

Updated

7 years ago
Blocks: 716598
(Reporter)

Updated

7 years ago
No longer blocks: 702032
(Reporter)

Comment 7

6 years ago
I haven't really seen this in a long time, so this can probably be WONTFIX.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.