Closed Bug 1247359 Opened 4 years ago Closed 4 years ago

micro-optimize the common case of String{Begins,End}With

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

Details

Attachments

(1 file)

StringBeginsWith (resp. StringEndsWith) takes a defaulted
nsStringComparator object for doing comparisons.  The flexibility this
affords is great, but the cost is not: nsStringComparator has virtual
methods, so initializing that defaulted object (at every callsite)
requires a temporary object whose vtable must be initialized.

Since the overwhemingly common case is to use the default comparator
anyway, we should not use defaulted arguments and instead provide the
default comparator/user-provided comparator cases as separate overloads.
This change eliminates the virtual call for the majority of callsites
and reduces codesize as well.
I don't particularly like duplicating all of this, but these functions should
probably be methods on nsTSubstring anyway, and there's already some precedent
for the copied code, so...meh?

Eric said he'd like to do some more XPCOM reviewing, maybe he has clever ideas
for addressing this.  Measurements on x86-64 Linux indicate libxul's text
section is ~5K smaller from this change.
Attachment #8718014 - Flags: review?(erahm)
Comment on attachment 8718014 [details] [diff] [review]
micro-optimize the common case of String{Begins,End}With

Review of attachment 8718014 [details] [diff] [review]:
-----------------------------------------------------------------

I agree the duplication is a shame, but it follows the standard set in Substring as well. Personally I find the duplication of nsAString and nsACString variants more egregious, but there's not much we can do about that.
Attachment #8718014 - Flags: review?(erahm) → review+
https://hg.mozilla.org/mozilla-central/rev/e4afb9798bcb
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.