| Summary: | Add fast path for id/name/style/class in getAttribute/setAttribute bindings. | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Andreas Kling <kling> | ||||
| Component: | WebCore JavaScript | Assignee: | Andreas Kling <kling> | ||||
| Status: | RESOLVED INVALID | ||||||
| Severity: | Normal | CC: | cdumez, cmarcelo, commit-queue, dongseong.hwang, esprehn+autocc, kangil.han, kling, kondapallykalyan, rniwa | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Andreas Kling
2014-03-06 17:28:21 PST
Created attachment 226068 [details]
Patch
Can we make sure we haven't regressed other attar accessed significantly with this? Hi, I'm cherry-picking this to Blink, https://codereview.chromium.org/189483004/ In the bug, I measured performance. Both were run on my Ivy Bridge laptop. PerformanceTests/Bindings/get-attribute.html -> 16% improvement Before : Avg get-attribute: 273.565932runs/s After : Avg get-attribute: 317.818405runs/s PerformanceTests/Bindings/set-attribute.html -> 77% improvement Before : Avg set-attribute: 476.101349runs/s After : Avg set-attribute: 847.042662runs/s Comment on attachment 226068 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226068&action=review > Source/WebCore/dom/Element.cpp:954 > +void Element::bindingsSetAttribute(const String& localName, const String& newValue, ExceptionCode& ec) Could I ask why you used String& instead of AtomicString&? Blink gave up cherry-picking this patch because it causes regression of other attributes significantly (about 50%).
getAttribute("id") -> 11% improvement
Before : Avg get-attribute: 329.751776runs/s
After : Avg get-attribute: 367.252965runs/s
getAttribute("group") -> 51% regression
Before : Avg get-attribute-href: 329.339992runs/s
After : Avg get-attribute-href: 159.540438runs/s
setAttribute("id") -> 19% improvement
Before : Avg set-attribute: 809.784217runs/s
After : Avg set-attribute: 965.404519runs/s
setAttribute("group") -> 45% regression
Avg set-attribute-href: 756.714409runs/s
Avg set-attribute-href: 415.186785runs/s
(In reply to comment #5) > Blink gave up cherry-picking this patch because it causes regression of other attributes significantly (about 50%). > > getAttribute("id") -> 11% improvement > Before : Avg get-attribute: 329.751776runs/s > After : Avg get-attribute: 367.252965runs/s > > getAttribute("group") -> 51% regression > Before : Avg get-attribute-href: 329.339992runs/s > After : Avg get-attribute-href: 159.540438runs/s > > setAttribute("id") -> 19% improvement > Before : Avg set-attribute: 809.784217runs/s > After : Avg set-attribute: 965.404519runs/s > > setAttribute("group") -> 45% regression > Avg set-attribute-href: 756.714409runs/s > Avg set-attribute-href: 415.186785runs/s Dongseong, are those the results with Blink or the results with WebKit? Results may be different. Note that we didn't land this patch. Oh this. We don't need this now that we construct perfect strings in JSC. |