1390349 - ContentEventHandler::GetFlatTextLengthInRange() takes a lot of time under IMEContentObserver::CharacterDataChanged()

Status: RESOLVED DUPLICATE of bug 1375502

(Core :: DOM: Events, enhancement, P2)

Description

[(no longer active)]

https://searchfox.org/mozilla-central/rev/e5b13e6224dbe3182050cf442608c4cb6a8c5c55/dom/events/ContentEventHandler.cpp#2689

This function alone in my perf profiles of Speedometer takes about 6% of the total time of HTMLInputElement::SetValueInternal().

Masayuki, is there anything we can do to improve the performance here?  I'm not very familiar with this code, but it seems like there is a lot of very low hanging fruit here.

For example, we create a super expensive nsRange object here: <https://searchfox.org/mozilla-central/rev/e5b13e6224dbe3182050cf442608c4cb6a8c5c55/dom/events/ContentEventHandler.cpp#2737>

Only to extract the common ancestor, start and end node and index here <https://searchfox.org/mozilla-central/rev/e5b13e6224dbe3182050cf442608c4cb6a8c5c55/dom/base/nsContentIterator.cpp#310>.  Could we not add an API to nsIContentIterator to pass this information directly and avoid creating an nsRange object?

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I guess that about the range cost, I'll be back soon to bug 1375502.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Oops, I meant, I guess that almost all of the cost are creating and initializing the nsRange. And about the range cost, I'll be back soon. Otherwise, if nsContentIterator takes a lot of cost, I have no idea for now (except creating stack class of nsContentIterator).

Comment 3

[(no longer active)]

Cool.  I think it makes sense to wait for bug 1375502 and reprofile after...

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm, only ~5% faster on the testcase after fixing bug 1375502. Perhaps, it's bad that content iterator is allocated in the heap and/or it still has some expensive check (but I have no idea how to reduce or skip the check cost smartly).

Comment 5

[(no longer active)]

Can you please post a profile with the patches of that bug applied?

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Here is (with opt (non-pgo) build):
https://perf-html.io/public/cb6309030188b9eacf9cb4ebd09a2043b4aff394/calltree/?hiddenThreads=&thread=6&threadOrder=0-2-3-4-6-1-5

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Do you still see something bad in IMEContentObserver or ContentEventHandler?

Comment 8

[(no longer active)]

No, this is all I see under HTMLInputElement::SetValueInternal() in perf now:

-   19.36%     0.07%  Web Content  libxul.so  [.] mozilla::dom::HTMLInputElement::SetValueInternal ▒
   - 19.29% mozilla::dom::HTMLInputElement::SetValueInternal                                       ▒
      - 17.71% nsTextEditorState::SetValue                                                         ▒
         - 10.92% mozilla::TextEditor::SetText                                                     ▒
            - 5.98% mozilla::TextEditRules::WillSetText                                            ▒
               - 4.99% mozilla::EditorBase::SetTextImpl                                            ▒
                  - 4.67% nsGenericDOMDataNode::SetData                                            ▒
                     - 4.61% nsGenericDOMDataNode::SetTextInternal                                 ▒
                        - 2.13% nsNodeUtils::CharacterDataChanged                                  ▒
                           - 1.56% mozilla::PresShell::CharacterDataChanged                        ▒
                              - 1.36% nsCSSFrameConstructor::CharacterDataChanged                  ▒
                                 - 0.95% nsTextFrame::CharacterDataChanged                         ▒
                                      0.62% mozilla::PresShell::FrameNeedsReflow                   ▒
                          0.57% nsTextFragment::SetTo                                              ▒
            - 2.78% mozilla::TextEditor::EndOperation                                              ▒
               - 2.64% mozilla::TextEditRules::AfterEdit                                           ▒
                  - 2.17% mozilla::TextEditRules::CollapseSelectionToTrailingBRIfNeeded            ▒
                     - 1.81% mozilla::EditorBase::CollapseSelectionToEnd                           ▒
                        - 1.69% mozilla::dom::Selection::Collapse                                  ▒
                           - 1.49% mozilla::dom::Selection::Collapse                               ▒
                                0.56% mozilla::dom::Selection::AddItemInternal                     ▒
              0.55% mozilla::EditorBase::EndPlaceholderTransaction                                 ▒
         - 1.66% mozilla::dom::Selection::EndBatchChangesInternal                                  ▒
            - 1.55% nsFrameSelection::EndBatchChanges                                              ▒
                 1.20% mozilla::dom::Selection::NotifySelectionListeners                           ▒
         - 0.96% mozilla::dom::Selection::RemoveAllRangesTemporarily                               ▒
            - 0.71% mozilla::dom::Selection::RemoveAllRanges                                       ▒
                 0.53% mozilla::dom::Selection::Clear                                              ▒
         - 0.90% nsTextControlFrame::SetValueChanged                                               ▒
              0.50% nsTextEditorState::UpdateOverlayTextVisibility                                 ▒

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Thanks! I'm still reading a lot for finding what we can do in Selection.