1941140 - Editor code wastes time doing allocation during teardown (`TextControlState::UnbindFromFrame` calls `HTMLInputElement::GetControllers` which allocates)

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Daniel Holbert [:dholbert] (PTO until 2/24)]

See bug 1939344 comment 3 -- that bug has a testcase with thousands of <input> elements, and when we tear down the document, we waste a bunch of time doing allocation. Can we optimize the TextControlState::UnbindFromFrame cleanup code to avoid doing that?

Profile link:
https://share.firefox.dev/3WhSLqi

In that profile, we're spending 426 samples (roughly 426ms) doing allocation while we're destroying stuff, in this backtrace:

malloc [memory/build/malloc_decls.h]
mozilla::StringBuffer::Alloc(unsigned long, mozilla::Maybe<unsigned long>) [mfbt/StringBuffer.h]
nsTSubstring<char>::StartBulkWriteImpl(unsigned long, unsigned long, bool, unsigned long, unsigned long, unsigned long) [xpcom/string/nsTSubstring.cpp]
nsTSubstring<char>::Assign(char const*, unsigned long, std::nothrow_t const&) [xpcom/string/nsTSubstring.cpp]
nsTSubstring<char>::Assign(nsTSubstring<char> const&, std::nothrow_t const&) [xpcom/string/nsTSubstring.cpp]
nsTSubstring<char>::Assign(nsTSubstring<char> const&) [xpcom/string/nsTSubstring.cpp]
nsTString<char>::nsTString(nsTSubstring<char> const&) [xpcom/string/nsTString.h]
nsCStringHashKey::nsCStringHashKey(nsTSubstring<char> const*) [xpcom/ds/nsHashKeys.h]
nsBaseHashtableET<nsCStringHashKey, nsCOMPtr<nsIControllerCommand> >::nsBaseHashtableET<nsIControllerCommand*&>(nsIControllerCommand*&) [xpcom/ds/nsBaseHashtable.h]
nsTHashtable<nsBaseHashtableET<nsCStringHashKey, nsCOMPtr<nsIControllerCommand> > >::EntryHandle::InsertInternal<nsIControllerCommand*&>(nsIControllerCommand*&)::{lambda(PLDHashEntryHdr*)#1}::operator()(PLDHashEntryHdr*) const [xpcom/ds/nsTHashtable.h]
PLDHashTable::EntryHandle::Insert<nsTHashtable<nsBaseHashtableET<nsCStringHashKey, nsCOMPtr<nsIControllerCommand> > >::EntryHandle::InsertInternal<nsIControllerCommand*&>(nsIControllerCommand*&)::{lambda(PLDHashEntryHdr*)#1}>(nsTHashtable<nsBaseHashtableET<nsCStringHashKey, nsCOMPtr<nsIControllerCommand> > >::EntryHandle::InsertInternal<nsIControllerCommand*&>(nsIControllerCommand*&)::{lambda(PLDHashEntryHdr*)#1}&&) [xpcom/ds/PLDHashTable.h]
nsTHashtable<nsBaseHashtableET<nsCStringHashKey, nsCOMPtr<nsIControllerCommand> > >::EntryHandle::InsertInternal<nsIControllerCommand*&>(nsIControllerCommand*&) [xpcom/ds/nsTHashtable.h]
nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::EntryHandle::Insert<nsIControllerCommand*&>(nsIControllerCommand*&) [xpcom/ds/nsBaseHashtable.h]
nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::EntryHandle::InsertOrUpdate<nsIControllerCommand*&>(nsIControllerCommand*&) [xpcom/ds/nsBaseHashtable.h]
nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}::operator()<nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::EntryHandle>(nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::EntryHandle) const [xpcom/ds/nsBaseHashtable.h]
nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::WithEntryHandle<nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}>(nsTSubstring<char> const&, nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}&&)::{lambda(auto:1)#1}::operator()<nsTHashtable<nsBaseHashtableET<nsCStringHashKey, nsCOMPtr<nsIControllerCommand> > >::EntryHandle>(nsTHashtable<nsBaseHashtableET<nsCStringHashKey, nsCOMPtr<nsIControllerCommand> > >::EntryHandle) const [xpcom/ds/nsBaseHashtable.h]
nsTHashtable<nsBaseHashtableET<nsCStringHashKey, nsCOMPtr<nsIControllerCommand> > >::WithEntryHandle<nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::WithEntryHandle<nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}>(nsTSubstring<char> const&, nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}&&)::{lambda(auto:1)#1}>(nsTSubstring<char> const&, nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::WithEntryHandle<nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}>(nsTSubstring<char> const&, nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}&&)::{lambda(auto:1)#1}&&)::{lambda(auto:1)#1}::operator()<PLDHashTable::EntryHandle>(PLDHashTable::EntryHandle) const [xpcom/ds/nsTHashtable.h]
PLDHashTable::WithEntryHandle<nsTHashtable<nsBaseHashtableET<nsCStringHashKey, nsCOMPtr<nsIControllerCommand> > >::WithEntryHandle<nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::WithEntryHandle<nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}>(nsTSubstring<char> const&, nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}&&)::{lambda(auto:1)#1}>(nsTSubstring<char> const&, nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::WithEntryHandle<nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}>(nsTSubstring<char> const&, nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}&&)::{lambda(auto:1)#1}&&)::{lambda(auto:1)#1}>(void const*, nsTHashtable<nsBaseHashtableET<nsCStringHashKey, nsCOMPtr<nsIControllerCommand> > >::WithEntryHandle<nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::WithEntryHandle<nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}>(nsTSubstring<char> const&, nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}&&)::{lambda(auto:1)#1}>(nsTSubstring<char> const&, nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::WithEntryHandle<nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}>(nsTSubstring<char> const&, nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}&&)::{lambda(auto:1)#1}&&)::{lambda(auto:1)#1}&&) [xpcom/ds/PLDHashTable.h]
nsTHashtable<nsBaseHashtableET<nsCStringHashKey, nsCOMPtr<nsIControllerCommand> > >::WithEntryHandle<nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::WithEntryHandle<nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}>(nsTSubstring<char> const&, nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}&&)::{lambda(auto:1)#1}>(nsTSubstring<char> const&, nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::WithEntryHandle<nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}>(nsTSubstring<char> const&, nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}&&)::{lambda(auto:1)#1}&&) [xpcom/ds/nsTHashtable.h]
nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::WithEntryHandle<nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}>(nsTSubstring<char> const&, nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&)::{lambda(auto:1)#1}&&) [xpcom/ds/nsBaseHashtable.h]
nsBaseHashtable<nsCStringHashKey, nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*, nsDefaultConverter<nsCOMPtr<nsIControllerCommand>, nsIControllerCommand*> >::InsertOrUpdate<nsIControllerCommand*&>(nsTSubstring<char> const&, nsIControllerCommand*&) [xpcom/ds/nsBaseHashtable.h]
nsControllerCommandTable::RegisterCommand(char const*, nsIControllerCommand*) [dom/commandhandler/nsControllerCommandTable.cpp]
mozilla::EditorController::RegisterEditorCommands(nsControllerCommandTable*) [editor/libeditor/EditorController.cpp]
CreateCommandTableWithCommands(nsresult (*)(nsControllerCommandTable*)) [dom/commandhandler/nsControllerCommandTable.cpp]
nsControllerCommandTable::CreateEditorCommandTable() [dom/commandhandler/nsControllerCommandTable.cpp]
CreateControllerWithSingletonCommandTable(already_AddRefed<nsControllerCommandTable> (*)()) [dom/commandhandler/nsBaseCommandController.cpp]
nsBaseCommandController::CreateEditorController() [dom/commandhandler/nsBaseCommandController.cpp]
mozilla::dom::HTMLInputElement::GetControllers(mozilla::ErrorResult&) [dom/html/HTMLInputElement.cpp]
mozilla::dom::HTMLInputElement::GetControllers(nsIControllers**) [dom/html/HTMLInputElement.cpp]
mozilla::TextControlState::UnbindFromFrame(nsTextControlFrame*) [dom/html/TextControlState.cpp]
nsTextControlFrame::Destroy(mozilla::FrameDestroyContext&) [layout/forms/nsTextControlFrame.cpp]

Comment 1

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

Indeed, creating new controller at cleaning up does not make sense.

Comment 2

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

Attachment: Bug 1941140 - Make `TextControlState::UnbindFromFrame` clean up controllers if and only if they are exist r=edgar!

HTMLInputElement::GetControllers() and HTMLTextAreaElement::GetControllers()
initializes their controllers before returning the nsIControllers instance
[1][2]. Typically, they are initialized at creating their TextEditor
instance is initialized [3]. A TextEditor for an HTMLInputElement is
initialized when it gets focus first time after the nsTextControlFrame is
created [4][5]. Therefore, when TextControlState::UnbindFromFrame is called,
the element may not have controllers yet. So, the elements should have simple
getters of their controllers and UnbindFromFrame should use them instead.

1. https://searchfox.org/mozilla-central/rev/eb80f267aabbf90ac66ebbed77b6d33592eb2be0/dom/html/HTMLInputElement.cpp#5658-5660
2. https://searchfox.org/mozilla-central/rev/eb80f267aabbf90ac66ebbed77b6d33592eb2be0/dom/html/HTMLTextAreaElement.cpp#502-503
3. https://searchfox.org/mozilla-central/rev/eb80f267aabbf90ac66ebbed77b6d33592eb2be0/dom/html/TextControlState.cpp#1826-1827,1832,1837-1838,1857-1858
4. https://searchfox.org/mozilla-central/rev/eb80f267aabbf90ac66ebbed77b6d33592eb2be0/layout/forms/nsTextControlFrame.cpp#447
5. https://searchfox.org/mozilla-central/rev/eb80f267aabbf90ac66ebbed77b6d33592eb2be0/dom/html/TextControlState.cpp#1640-1641,1655-1656

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de25a04c9bd8
Make `TextControlState::UnbindFromFrame` clean up controllers if and only if they are exist r=edgar

Comment 4

[amarc]

https://hg.mozilla.org/mozilla-central/rev/de25a04c9bd8