1365874 - Selection->Collapse is slow

Status: NEW

(Core :: DOM: Selection, defect)

Description

[Makoto Kato [:m_kato]]

On bug 1346723's benchmark, Selection->Collapse() spends more time.  input.value setter for focused element has to  set caret after updating value.

Actually, 6% of nsTextEditorStaet::SetValue is Selection::Collapse at least.  After fixing bug 1358025, more 20% will be into this.

Comment 1

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

A timely topic. I've seen same result when I check usual text typing result.

Selection::Collapse() calls both nsRange::SetEnd() and nsRange::SetStart():
https://searchfox.org/mozilla-central/rev/f55349994fdac101d121b11dac769f3f17fbec4b/layout/generic/nsSelection.cpp#5247,5252

This is redundant and causes a call of nsContentUtils::ComparePoints():
https://searchfox.org/mozilla-central/rev/f55349994fdac101d121b11dac769f3f17fbec4b/dom/base/nsRange.cpp#1233,1247

It seems that just omitting the call of nsRange::SetEnd() fixes this bug.

Comment 2

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

I profiled this again with attachment 8848015 [details].

Then, I see that the large part of the case is creating, destroying ranges. If old range is not grabbed by the others, i.e., the refcount is 1, we could reuse the instance. Then, we can skip common ancestor handling that is also expensive.

Additionally, Selection::setFrames() creates two content iterators. If we can omit to create them, we can improve the performance too.

Comment 3

[(no longer active)]

Hi Masayuki, I see that all of the dependencies of this bug have been closed.  What's the current status of this bug?

Comment 4

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

I guess that we can improve this more if I'll succeed to redesign around nsRange (not yet file a bug) for bug 1367744 (Selection::Extend). But I'm struggling with it for a week due to a lot of oranges. So, currently, I say only, I'm not sure for now.

Comment 5

[(no longer active)]

OK, thanks for the update. :-)

Comment 6

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

Resetting assignee which I don't work on in this several months.