Bug 90273

Summary: Optimize Element::hasAttribute() by replacing String with AtomicString
Product: WebKit Reporter: Kentaro Hara <haraken>
Component: DOMAssignee: Kentaro Hara <haraken>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, cmarcelo, rniwa, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 90246    
Attachments:
Description Flags
Performance test
none
Patch
webkit.review.bot: commit-queue-
patch for landing none

Kentaro Hara
Reported 2012-06-29 04:58:38 PDT
Based on the observation described in this ChangeLog (http://trac.webkit.org/changeset/121439), the performance of Element::hasAttribute() can be optimized by replacing String with AtomicString.
Attachments
Performance test (1.55 KB, text/html)
2012-06-29 05:10 PDT, Kentaro Hara 		no flags
Details
Patch (3.43 KB, patch)
2012-06-29 05:12 PDT, Kentaro Hara 		webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
patch for landing (3.33 KB, patch)
2012-08-07 19:20 PDT, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Kentaro Hara
Comment 1 2012-06-29 05:10:30 PDT
Created attachment 150144 [details] Performance test
Kentaro Hara
Comment 2 2012-06-29 05:12:16 PDT
Created attachment 150146 [details] Patch
Adam Barth
Comment 3 2012-07-27 01:04:22 PDT
Comment on attachment 150146 [details] Patch Based on further discussion in #webkit, my current understanding is that ap is ok with these patches. I'm going to r+ them, but please give ap a chance to respond before landing.
Darin Adler
Comment 4 2012-07-27 07:08:12 PDT
Comment on attachment 150146 [details] Patch This is faster in some cases. But a case I’d expect this makes slower is when the passed-in attribute name is uppercase. This adds a lookup in the atomic string table of the not-yet-lowercased string.
Kentaro Hara
Comment 5 2012-08-07 18:21:47 PDT
Comment on attachment 150146 [details] Patch Looks like no strong objection, let me commit it.
WebKit Review Bot
Comment 6 2012-08-07 18:49:33 PDT
Comment on attachment 150146 [details] Patch Rejecting attachment 150146 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: cceeded at 1 with fuzz 3. patching file Source/WebCore/dom/Element.cpp Hunk #1 succeeded at 1532 with fuzz 2 (offset 38 lines). Hunk #2 FAILED at 1502. 1 out of 2 hunks FAILED -- saving rejects to file Source/WebCore/dom/Element.cpp.rej patching file Source/WebCore/dom/Element.h Hunk #1 succeeded at 141 with fuzz 1 (offset 2 lines). Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Adam Barth']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/13459033
Kentaro Hara
Comment 7 2012-08-07 19:20:20 PDT
Created attachment 157086 [details] patch for landing
WebKit Review Bot
Comment 8 2012-08-08 01:43:38 PDT
Comment on attachment 157086 [details] patch for landing Clearing flags on attachment: 157086 Committed r125009: <http://trac.webkit.org/changeset/125009>
Note You need to log in before you can comment on or make changes to this bug.