Closed Bug 1612085 Opened 5 years ago Closed 5 years ago

Remove `Selection::mCachedRange`

Categories

(Core :: DOM: Selection, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla74
Tracking Status
firefox74 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

Attachments

(3 files)

In bug 1393816, I added Selection::mCachedRange to avoid reallocation cost in specific path. And in bug 1479972, I made it more complicated. They are necessary hack for Quantum Flow, but they are really ugly and blocks understanding of new developers.

Now, I have better idea.

  1. Hide constructor of nsRange and make all users create it with factory method.
  2. Make factory method cache deleting instance and make the factory method reuse the cached instances.
  3. Remove the hacks.

Currently, I'm testing performance of test builds.

Previously, I added Selection::mCachedRange to save allocation cost of
nsRange. However, with the previous patch, we don't need the hack anymore
since ranges removed by Selection::RemoveAllRanges() are always kept in
the global cache of nsRange. Therefore, we can remove the ugly hack right
now.

Depends on D61238

This patch makes nsRange::Create() reuse its instances automatically.
It's difficult to consider the limit of cache since nsRange instance is
created not so many in most cases, but only Find and Spellchecker sometimes
create too many instances.

Depends on D61237

nsRange instances are allocated a lot in the heap especially by editor and
spellchecker. The allocation cost is too bad for benchmarks. Therefore,
we should reuse released instances as far as possible. For managing it in
static factory methods of nsRange, we need to hide nsRange constructor.

Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/889491a898dc part 1: Hide constructor of `nsRange` r=smaug https://hg.mozilla.org/integration/autoland/rev/cb9253c24727 part 2: Make `nsRange` instances reused r=smaug https://hg.mozilla.org/integration/autoland/rev/9e67aa2994ac part 3: Remove dirty hack of `Selection::mCachedRange` r=smaug
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: