74830 - 50% of time in SelectorMatches caused by unused variable

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Daniel Bratell]

SelectorMatches shows up with a couple of percent of the time on big pages. 
Maybe it's the same with small pages, but I haven't looked at them. In 
SelectorMatches 35% of the time comes from the constructor of a nsAutoString, 
and 20% from the destructor. The funny thing is that the only nsAutoString there 
(named buffer) isn't even used. 

patch:

  static PRBool SelectorMatches(SelectorMatchesData &data,
                                nsCSSSelector* aSelector,
                                PRBool aTestState,
                                PRInt8 aNegationIndex) 
 
  {
    // if we are dealing with negations, reverse the values of PR_TRUE and 
PR_FALSE
    PRBool  localFalse = PRBool(0 < aNegationIndex);
    PRBool  localTrue = PRBool(0 == aNegationIndex);
    PRBool  checkType = (0 == aNegationIndex) || (1 < aNegationIndex);
    PRBool  result = localFalse;
-   nsAutoString buffer;
 
    // Bail out early if we can. Do not perform the test if aNegationIndex==1
    // because it then contains only negated IDs, classes, attributes and 
pseudo-
    // classes
   if (checkType) {


attinasi, can you take this one? It will only save like 1-2% of the time on the 
table stress cases and the jdk index, but it's the most easy 1-2% I've seen in a 
long time. :-)

Comment 1

[Marc Attinasi]

Thanks Daniel - I surely will take this! 

My belief is: SelectorMatches must be as fast as possible.

Comment 2

[Marc Attinasi]

This snuck in when glazman made the :not changes.

Here is the patch suggested by Daniel Bratell. sr=attinasi, looking for r= from 
glazman or anyone else (very simple changes like this should not require such 
process, but whatever).

Index: nsCSSStyleSheet.cpp
===================================================================
RCS file: /cvsroot/mozilla/content/html/style/src/nsCSSStyleSheet.cpp,v
retrieving revision 3.152
diff -u -r3.152 nsCSSStyleSheet.cpp
--- nsCSSStyleSheet.cpp 2001/03/30 10:20:56     3.152
+++ nsCSSStyleSheet.cpp 2001/04/05 05:31:20
@@ -2940,7 +2940,6 @@
   PRBool  localTrue = PRBool(0 == aNegationIndex);
   PRBool  checkType = (0 == aNegationIndex) || (1 < aNegationIndex);
   PRBool  result = localFalse;
-  nsAutoString buffer;

   // Bail out early if we can. Do not perform the test if aNegationIndex==1
   // because it then contains only negated IDs, classes, attributes and pseudo-

Comment 3

[Blake Ross]

r=blake

Comment 4

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Oh my goodness... Very good catch, Daniel, tack så mycket...

r=glazman

Anyway, fixing 72302 brought to the light bugs in :not() that were invisible
before precisely because of 72302. I'll have to reopen 71647.

Comment 5

[Brendan Eich [:brendan]]

attinasi: just do it.  The rules/tips in
http://www.mozilla.org/hacking/reviewers.html are means to an end.  There's a
"basic speed law", and if you are driving no faster than is safe, you won't get
a ticket for checking in a fix like this with only sr= and no r=.

/be

Comment 6

[Marc Attinasi]

Fix checked in: nsCSSStyleSheet.cpp v3.153