Closed Bug 596501 Opened 9 years ago Closed 2 years ago

Make TextEditor::OutputToString faster

Categories

(Core :: DOM: Editor, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: bzbarsky, Assigned: masayuki)

References

(Blocks 2 open bugs)

Details

(Keywords: perf, Whiteboard: [post-2.0])

Attachments

(1 file)

For the typical case (not password input, not wrap="hard" textarea, etc), we should be able to just hand back the string inside our one textnode, I would think.
Ehsan, do you have time to take a look at this?
I'm not sure I understand what you're proposing here.  Isn't that what we already do, through nsDocumentEncoder?
Yes, with giant amounts of overhead.  I'm proposing we skip the overhead.
(In reply to comment #3)
> Yes, with giant amounts of overhead.  I'm proposing we skip the overhead.

Do you have a profile on what takes too much time in this process?
Sure thing.  On the testcase in bug 190147, the time under OutputToString breaks down as follows (all percentages are of time under OutputToString; "in" means in the function, "under" means in the function or some function it calls):

4% setting up the document encoder, QIs, etc getting to call
   nsPlainTextSerializer::AppendText.
3% FindCharInset called from Appendtext
3% nsDependentSubstring::Rebind called from AppendText
2% nsString::AssignWithConversion called from AppendText
2% in AppendText itself
4% in nsPlainTextSerializer::DoAddLeaf.
48% under nsPlainTextSerializer::EndLine (calling Replace, calling SetLength,
    calling OutputQuotesAndIndent, etc) (why are the Replace and SetLength
    calls happening at all?)
4% in nsPlainTextSerializer::Write
3% FindCharInSet called from Write()
7% nsAString::Assign called from Write()
19% under nsPlaintextSerializer::AddToLine; almost all of this is calling
    Replace(); again not sure why we need this.
OK, sold!
Assignee: nobody → ehsan
Keywords: perf
I'm probably going to take a look at this post Firefox 4, so if someone else wants to steal it from me before then, I won't call the cops! ;-)
It'd still be nice to understand why those Replace and so forth happen, btw... and especially why they actually resize the strings!
Whiteboard: [post-2.0]
Blocks: 1346723
I'm not actively working on this...
Assignee: ehsan → nobody
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Summary: Make nsPlaintextEditor::OutputToString faster → Make TextEditor::OutputToString faster
I guess that this performance issue is not so appear in profiles dynamically. However, this is used by autocomplete at every type, caching the value in nsTextEditorState and caching change event when moving focus to editor.

So, some users could feel like this bug as "Firefox responds not so quickly at typing something". I'm not sure how to detect such similar issues.
The patch improves the score of attachment 8848015 [details] about 10%.
Comment on attachment 8879213 [details]
Bug 596501 TextEditRules::WillOutputText() should handle itself if it can return only the text of first text node

https://reviewboard.mozilla.org/r/150536/#review155446
Attachment #8879213 - Flags: review?(m_kato) → review+
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/36a05246c18c
TextEditRules::WillOutputText() should handle itself if it can return only the text of first text node r=m_kato
https://hg.mozilla.org/mozilla-central/rev/36a05246c18c
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.