If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Make TextEditor::OutputToString faster

RESOLVED FIXED in Firefox 56

Status

()

Core
Editor
RESOLVED FIXED
7 years ago
3 months ago

People

(Reporter: bz, Assigned: masayuki)

Tracking

(Blocks: 2 bugs, {perf})

Trunk
mozilla56
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [post-2.0])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

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]

Updated

5 months ago
Blocks: 1346723
I'm not actively working on this...
Assignee: ehsan → nobody
(Assignee)

Updated

3 months ago
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6ecbc3b280b47750ed80f36a2d642a9e8d96882f
https://treeherder.mozilla.org/#/jobs?repo=try&revision=af8136e4fa88392fba6f02f44231c65437857fbf
(Assignee)

Updated

3 months ago
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.
Comment hidden (mozreview-request)
The patch improves the score of attachment 8848015 [details] about 10%.

Comment 15

3 months ago
mozreview-review
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+

Comment 16

3 months ago
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

Comment 17

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/36a05246c18c
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.