1726570 - Accelerate nsFind by caching the IsCombiningDiacritic result for each Unicode codepoint

Status: RESOLVED FIXED

(Core :: Find Backend, enhancement)

Description

[Jonathan Kew [:jfkthame]]

Following on from bug 1725555 comment 5, the last profile there shows that looking up mozilla::unicode::IsCombiningDiacritic for each character as we search is a significant cost. This function currently does an ICU property lookup.

We can significantly speed this up by caching the exact result we care about (based on several comparisons of the CombiningClass property value) in a gfxSparseBitSet, which compactly records a single boolean flag for each codepoint, and doesn't waste space on the large areas of the code space where there are no true entries.

Comparing profiles of searching the log file mentioned in bug 1725555 for the text "Task Finished" in a local opt build, it currently takes around 830-850ms for nsFind::Find to search the entire document:
https://share.firefox.dev/3Db9jFg

With a patch to cache the IsCombiningDiacritic value, this is reduced to around 570-610ms:
https://share.firefox.dev/3j1KSSI

The gfxSparseBitSet that is needed to cache this flag for the whole Unicode range only takes around 3Kbytes, so the footprint is negligible. There's some one-time cost to initializing it, which is around 10ms on my machine (though obviously it'll be more on slower systems).

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1726570 - Accelerate nsFind by precomputing a const SharedBitSet for IsCombiningDiacritic. r=emilio

No user-visible change to behavior, except that searching a huge document
becomes slightly quicker.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a93cff68de74
Accelerate nsFind by precomputing a const SharedBitSet for IsCombiningDiacritic. r=emilio

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2fd9e81cee3c
Fix lint failures. a-lint-fix. CLOSED TREE

Comment 4

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/a93cff68de74
https://hg.mozilla.org/mozilla-central/rev/2fd9e81cee3c

Comment 5

[Mike Hommey [:glandium]]

Jonathan, is there a specific reason you made the GeneratedFile use force=True?

Comment 6

[Jonathan Kew [:jfkthame]]

(In reply to Mike Hommey [:glandium] from comment #5)

Jonathan, is there a specific reason you made the GeneratedFile use force=True?

I'm not sure at this point, sorry; I have a vague recollection of encountering some kind of build failure while working on this, so maybe that was a fix/workaround for that? If you think it's redundant and everything builds OK without, I'd be fine with dropping it.