714830 - [CC] consider optimizing CC traversal of nsContentSink

Status: RESOLVED WONTFIX

(Core :: DOM: HTML Parser, defect)

Description

[Andrew McCreight [:mccr8]]

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.

Comment 1

[Andrew McCreight [:mccr8]]

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

[Olli Pettay [:smaug][[e-mail]]]

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.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

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

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Attachment: 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.

Comment 5

[Henri Sivonen (:hsivonen)]

(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).

Comment 6

[Henri Sivonen (:hsivonen)]

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

Comment 7

[Andrew McCreight [:mccr8]]

I haven't really seen this in a long time, so this can probably be WONTFIX.