132679 - optimize nsString::Equals* to be faster than Compare()

Status: RESOLVED INCOMPLETE

(Core :: XPCOM, defect, P2)

Description

[Alec Flett]

Ok, I've been wallowing around in nsStr*.cpp, and watching calls to
StrCompare2To1 and friends.. and basically I've decided we really need to
optimize nsString::Equals and friends (nsString::EqualsIgnoreCase, etc)

1) We need to make it check for length before any kind of comparison, so that if
lengths are unequal, then we immediately stop and proclaim the strings unequal
2) we need EqualsIgnoreCase to accept nsAFlatString&'s so that people can pass
in strings with length. I'm seeing literally hundreds (if not thousands) of
string comparisons like this in CSS:
if (str.EqualsIgnoreCase("inherit"))...

which would be greatly sped up by accepting nsAFlatStrings, and replacing this with 
if (str.EqualsIgnoreCase(NS_LITERAL_CSTRING("inherit"))...

I have no idea what kind of performance benefit this would have, but I sure see
a lot of calls to Compare2to1 :)

Comment 1

[Boris Zbarsky [:bzbarsky]]

doesn't nsString::Equals already check the length?  (nsAString::Equals certainly
does).

Also, do we want to just change nsString::EqualsIgnoreCase?  Or also to convert
those calls to use plain old Equals() with a comparator (that would involve
putting NS_LITERAL_STRING all over and paying the memory penalty (on
Windows/Mac), unfortunately).

Comment 2

[Alec Flett]

nah, in this case I think its better to have EqualsIgnoreCase - we don't want to
call into the intl libraries just to see if a string matches "inherit" or
something ascii-like like that.

I'm moving this out to moz1.1 for now because I'm pretty darn busy right now...
however in my tree I have conversion of much of nsStr*.cpp over to
nsAFlatString, which makes the EqualsIgnoreCase(const nsAFlatString& ) a whole
lot easier.

(and no, I didn't convert to nsASingleFragmentString..because it doesn't have
.get() or a similarly easy way to grab a pointer to the front of the fragment)

Comment 3

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

> (and no, I didn't convert to nsASingleFragmentString..because it doesn't have
> .get() or a similarly easy way to grab a pointer to the front of the fragment)

const PRUnichar* begin;
myASingleFragmentString.BeginReading(begin);

Comment 4

[Alec Flett]

Yes, I know :) My issue is that I've got about a million lines like

StrCompare(foo.mStr, bar.mUStr, ...); etc

or even more annoying

if (blah(foo.mStr,..)) {
  if (compare(bar.mStr...)) {
  } else if (compare(bar.mUStr, ...))

etc
what I'm getting at is I'd like a functional (as in lisp) way of getting to
this. Maybe I'll write an inline helper routine...

Comment 5

[Alec Flett]

woah, 1.1 came up quick. Throwing these bugs over to 1.2.. little or no work has
been done and there is no way that these bugs can be fixed in 1.1.

Comment 6

[Alec Flett]

pushing out to mozilla 1.3alpha for now. I might come back to this, or it might
just get fixed when I fix bug 156713

Comment 7

[Alec Flett]

moving non-critical 1.3alpha bugs to 1.4alpha

Comment 8

[Alec Flett]

mass moving lower risk 1.4alpha stuff to 1.4beta