Bug 87832 - [Performance] Optimize querySelector() by caching a CSSParser object on Document
Summary: [Performance] Optimize querySelector() by caching a CSSParser object on Document
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on:
Blocks: 87625
  Show dependency treegraph
 
Reported: 2012-05-30 01:09 PDT by Kentaro Hara
Modified: 2012-05-30 01:32 PDT (History)
2 users (show)

See Also:


Attachments
Patch [A] (3.13 KB, patch)
2012-05-30 01:17 PDT, Kentaro Hara
no flags Details | Formatted Diff | Diff
Patch [B] (4.41 KB, patch)
2012-05-30 01:19 PDT, Kentaro Hara
no flags Details | Formatted Diff | Diff
Patch [C] (5.06 KB, patch)
2012-05-30 01:28 PDT, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2012-05-30 01:09:27 PDT
Currently one of the bottlenecks of Node::querySelector() and Node::querySelectorAll() is to create a CSSParser object every time.

PassRefPtr<Element> Node::querySelector(const String& selectors, ExceptionCode& ec)
{
    CSSParser parser(document());  // This is a bottleneck!
    CSSSelectorList querySelectorList;
    parser.parseSelector(selectors, querySelectorList);
    ...;
}

We can avoid creating a CSSParser object by caching the CSSParser object on Document.

I'll upload a couple of proposed patches for performance comparison. After that I'll post some comments on the patches.
Comment 1 Kentaro Hara 2012-05-30 01:17:06 PDT
Created attachment 144745 [details]
Patch [A]
Comment 2 Kentaro Hara 2012-05-30 01:19:58 PDT
Created attachment 144747 [details]
Patch [B]
Comment 3 Kentaro Hara 2012-05-30 01:28:19 PDT
Created attachment 144751 [details]
Patch [C]
Comment 4 Antti Koivisto 2012-05-30 01:29:37 PDT
CSSParser has so far not been reusable. There might be state left from the previous runs that affect the results of the next runs.

I think you should try caching SelectorQueries first. That would avoid the need to spin up the parser repeatedly in the first place.
Comment 5 Kentaro Hara 2012-05-30 01:32:46 PDT
(In reply to comment #4)
> CSSParser has so far not been reusable. There might be state left from the previous runs that affect the results of the next runs.
> 
> I think you should try caching SelectorQueries first. That would avoid the need to spin up the parser repeatedly in the first place.

Sounds reasonable. Caching SelectorQueries would solve the CSSParser construction problem.