Bug 20176 - querySelectorAll id optimization no longer working
: querySelectorAll id optimization no longer working
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: HTML DOM
: 528+ (Nightly build)
: Macintosh Mac OS X 10.5
: P2 Normal
Assigned To: David Smith
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-07-25 17:00 PDT by David Smith
Modified: 2008-07-27 15:06 PDT (History)
0 users

See Also:


Attachments
Patch; Passes run-webkit-tests and doesn't regress http://ejohn.org/apps/selectortest/#target (2.40 KB, patch)
2008-07-25 17:06 PDT, David Smith
sam: review-
Details | Formatted Diff | Diff
patch (12.22 KB, patch)
2008-07-27 14:55 PDT, Sam Weinig
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Smith 2008-07-25 17:00:02 PDT
Regresses the time for the #speech5 test on webkit.org/perf/slickspeed from 1-3ms to 10-12+ ms.
Comment 1 David Smith 2008-07-25 17:06:07 PDT
Created attachment 22488 [details]
Patch; Passes run-webkit-tests and doesn't regress http://ejohn.org/apps/selectortest/#target
Comment 2 Eric Seidel 2008-07-26 22:41:41 PDT
Comment on attachment 22488 [details]
Patch; Passes run-webkit-tests and doesn't regress http://ejohn.org/apps/selectortest/#target

Looks good.
Comment 3 Sam Weinig 2008-07-26 22:51:03 PDT
Comment on attachment 22488 [details]
Patch; Passes run-webkit-tests and doesn't regress http://ejohn.org/apps/selectortest/#target

The check shouldn't be necessary, and can be made an ASSERT as far as I can tell.  It should also read querySelector->m_attr== idAttr, not querySelector->m_attr.localName() == idAttr.

I believe this will also break some uses of querySelector("#camalCase"); in quirks mode.  We probably need to disable the optimization in quirks mode methinks.
Comment 4 Sam Weinig 2008-07-27 14:55:26 PDT
Created attachment 22511 [details]
patch
Comment 5 Sam Weinig 2008-07-27 15:06:39 PDT
Fixed in r35406.