Bug 99428 - [Shadow DOM][V8] WebCore::V8DOMWindow::installPerContextProperties() is slow when shadowDOMEnabled flag is on.
Summary: [Shadow DOM][V8] WebCore::V8DOMWindow::installPerContextProperties() is slow ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hajime Morrita
URL:
Keywords:
Depends on:
Blocks: 72352
  Show dependency treegraph
 
Reported: 2012-10-16 00:29 PDT by Hajime Morrita
Modified: 2012-10-16 22:39 PDT (History)
5 users (show)

See Also:


Attachments
Patch (15.04 KB, patch)
2012-10-16 20:17 PDT, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch for landing (15.00 KB, patch)
2012-10-16 21:50 PDT, Hajime Morrita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hajime Morrita 2012-10-16 00:29:26 PDT
This upstreams http://code.google.com/p/chromium/issues/detail?id=155620
A profiling unveiled that this installPerContextProperties() is slower if the flag is on.
It makes sense since it injects extra DOMWindow properties at runtime.

My assumption is that installPerContextProperties() is called only a few times per page.
But there are some outliers in page cycler suite which consume 1.3% time in this function for each page load. 

I'll make some micro-benchmark for this.
Comment 1 Hajime Morrita 2012-10-16 00:33:51 PDT
Basically there are two approaches to fix this.

A. Stop using EnablePerContext: Considering Chromium make it available for the wild Web, 
    The Shadow DOM feature does no longer need to be contextual. 
B. Make EnablePerContext faster somehow. I have no concrete idea yet. But maybe we should do this
    for other features which uses this contextual switch mechanism.

My feeling is that we can land A first then attack B if possible.
Comment 2 Dimitri Glazkov (Google) 2012-10-16 07:35:38 PDT
(In reply to comment #1)
> My feeling is that we can land A first then attack B if possible.

That sounds good.
Comment 3 Hajime Morrita 2012-10-16 20:17:26 PDT
Created attachment 169079 [details]
Patch
Comment 4 Hajime Morrita 2012-10-16 21:50:46 PDT
Created attachment 169085 [details]
Patch for landing
Comment 5 WebKit Review Bot 2012-10-16 22:39:05 PDT
Comment on attachment 169085 [details]
Patch for landing

Clearing flags on attachment: 169085

Committed r131549: <http://trac.webkit.org/changeset/131549>
Comment 6 WebKit Review Bot 2012-10-16 22:39:09 PDT
All reviewed patches have been landed.  Closing bug.