1503928 - FontFamilyList::Contains does some slow string comparisons

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, enhancement)

Description

[Olli Pettay [:smaug][[e-mail]]]

https://perf-html.io/public/00cd9c8c3184cbc571e0b3a744477635a5905508/calltree/?file=profile_raptor-tp6-amazon-firefox%2Fraptor-tp6-amazon-firefox_pagecycle_1.profile&globalTrackOrder=0-1-2-3-4&hiddenGlobalTracks=1-2-4&invertCallstack&localTrackOrderByPid=3625-1-0~3748-0~3870-0~&range=1.9671_4.5774~4.0345_4.1193&search=ToLowercase&thread=4&v=3

nsAtomString is slow, could we use nsDependentAtomString somehow.
And LowerCaseEqualsLiteral


I don't know why the raptor profile shows this stuff so badly. Does Amazon really load so many fonts, or is this about comparing to local font names or what?

Comment 1

[Jonathan Kew [:jfkthame]]

AFAICS we can't use LowerCaseEqualsLiteral here, because neither of the strings concerned is a literal; and we can't use nsDependentAtomString because we're going to apply ToLowerCase to the string, so we need our own copy.

One micro-optimization we could do here would be to use nsAutoString for listname, to avoid a separate string allocation.

An alternative approach would be to use nsCaseInsensitiveStringComparator instead of applying ToLowerCase to the strings. (And then we could indeed use nsDependentAtomString.) Do you happen to know whether that's likely to be more or less performant than pre-lowercasing the strings, given that we're likely to be doing a series of comparisons against the same input string?

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

If we can avoid allocating string from atom, I would definitely expect it to be faster. Allocations are very slow.

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: Optimize FontFamilyList::Contains to avoid allocation and string-copying

OK, I instrumented a local (opt) build, and confirmed that (a) using a local nsAutoString is better than nsAtomString (as expected), because it avoids the extra allocation; and (b) better still is to use nsDependentAtomString and nsCaseInsensitiveStringComparator instead of lowercasing the two strings. Average duration of calls to Contains() in a brief test across a few sites went from 0.75µs (original) to 0.58µs (using nsAutoString) to 0.51µs (avoiding string copies and use case-insensitive comparator).

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/72b791de2f09
Optimize FontFamilyList::Contains to avoid allocation and string-copying. r=lsalzman

Comment 5

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/72b791de2f09