Bug 94990 - Share immutable ElementAttributeData between elements with identical attributes.
Summary: Share immutable ElementAttributeData between elements with identical attributes.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andreas Kling
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-08-24 17:53 PDT by Andreas Kling
Modified: 2012-09-04 05:34 PDT (History)
3 users (show)

See Also:


Attachments
Love letter to EWS (9.52 KB, patch)
2012-08-24 18:54 PDT, Andreas Kling
buildbot: commit-queue-
Details | Formatted Diff | Diff
Love letter to EWS + cheesy GCC buildfix (9.49 KB, patch)
2012-08-24 19:46 PDT, Andreas Kling
no flags Details | Formatted Diff | Diff
Patch!!11 (11.93 KB, patch)
2012-09-03 09:42 PDT, Andreas Kling
koivisto: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2012-08-24 17:53:48 PDT
<rdar://12141736>
Comment 1 Andreas Kling 2012-08-24 18:54:51 PDT
Created attachment 160542 [details]
Love letter to EWS
Comment 2 Build Bot 2012-08-24 19:18:41 PDT
Comment on attachment 160542 [details]
Love letter to EWS

Attachment 160542 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13574935
Comment 3 Early Warning System Bot 2012-08-24 19:21:22 PDT
Comment on attachment 160542 [details]
Love letter to EWS

Attachment 160542 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13591469
Comment 4 Andreas Kling 2012-08-24 19:46:38 PDT
Created attachment 160545 [details]
Love letter to EWS + cheesy GCC buildfix
Comment 5 Andreas Kling 2012-09-03 09:42:55 PDT
Created attachment 161939 [details]
Patch!!11
Comment 6 Andreas Kling 2012-09-03 11:21:11 PDT
I should note that there are a number of follow-up optimizations enabled by this, such as avoiding parsing the same style/class attributes more than once.
Comment 7 Antti Koivisto 2012-09-03 13:54:18 PDT
Comment on attachment 161939 [details]
Patch!!11

View in context: https://bugs.webkit.org/attachment.cgi?id=161939&action=review

Nice. r=me

> Source/WebCore/dom/Document.cpp:6178
> +class ImmutableAttributeDataCacheKey {

Document is super bloaty. Would be nice to factor this a separate file/class.

> Source/WebCore/dom/Document.cpp:6192
> +    bool operator!=(ImmutableAttributeDataCacheKey& other) const

const

> Source/WebCore/dom/Document.cpp:6235
> +    if (!cacheHash || cacheIterator->second)

The logic here is bit strange. I suspect it could be written in a nicer way.
Comment 8 Andreas Kling 2012-09-03 19:19:19 PDT
Committed r127438: <http://trac.webkit.org/changeset/127438>
Comment 9 Andreas Kling 2012-09-04 05:34:10 PDT
Perf-o-matic results are in, looks like a 3.6% improvement on Parser/html5-full-render:malloc

http://webkit-perf.appspot.com/graph.html#tests=[[6268923,2001,963028]]&sel=none&displayrange=7&datatype=running

Booyakasha!

@Antti: I agree that it can be factored better, I'll improve upon this in coming patches.