Bug 84103

Summary: [Performance][V8] Skip Isolate look-up to find StringCache
Product: WebKit Reporter: Kentaro Hara <haraken>
Component: WebCore JavaScriptAssignee: Kentaro Hara <haraken>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dglazkov, japhet, rniwa, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 84074    
Attachments:
Description Flags
Performance tests
none
Patch
none
Patch
none
Patch
none
Patch for relanding. It is revealed that the patch was innocent. none

Kentaro Hara
Reported 2012-04-16 17:19:59 PDT
StringCache is an Isolate-local data structure. Currently, V8 bindings need to look up an Isolate to find the StringCache. By using AccessorInfo.GetIsolate() or Arguments.GetIsolate(), we can remove the Isolate look-up.
Attachments
Performance tests (1.90 KB, text/html)
2012-04-16 17:21 PDT, Kentaro Hara 		no flags
Details
Patch (8.72 KB, patch)
2012-04-16 17:30 PDT, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
Patch (8.72 KB, patch)
2012-04-17 02:20 PDT, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
Patch (17.07 KB, patch)
2012-04-17 02:27 PDT, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
Patch for relanding. It is revealed that the patch was innocent. (17.06 KB, patch)
2012-04-23 09:54 PDT, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Kentaro Hara
Comment 1 2012-04-16 17:21:02 PDT
Created attachment 137440 [details] Performance tests
Kentaro Hara
Comment 2 2012-04-16 17:30:26 PDT
Created attachment 137441 [details] Patch
WebKit Review Bot
Comment 3 2012-04-16 18:08:39 PDT
Comment on attachment 137441 [details] Patch Attachment 137441 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12412665
Kentaro Hara
Comment 4 2012-04-17 02:20:40 PDT
Created attachment 137499 [details] Patch
Kentaro Hara
Comment 5 2012-04-17 02:24:18 PDT
Rolled chromium DEPS to r132474 and re-uploaded the patch. The patch requires V8 r11306.
Kentaro Hara
Comment 6 2012-04-17 02:27:43 PDT
Created attachment 137502 [details] Patch
Ryosuke Niwa
Comment 7 2012-04-17 09:54:54 PDT
Comment on attachment 137502 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=137502&action=review > Source/WebCore/ChangeLog:34 > + Performance test: https://bugs.webkit.org/attachment.cgi?id=137440 > + div.id: 30.70ns -> 26.70ns (+15%) > + div.className: 31.10ns -> 26.40ns (+18%) > + div.nodeName: 37.70ns -> 33.00ns (+14%) > + text.nodeValue: 31.40ns -> 25.90ns (+21%) > + text.textContent: 51.50ns -> 45.90ns (+12%) Should we add it as a performance test?
Kentaro Hara
Comment 8 2012-04-17 09:58:49 PDT
(In reply to comment #7) > > Source/WebCore/ChangeLog:34 > > + Performance test: https://bugs.webkit.org/attachment.cgi?id=137440 > > + div.id: 30.70ns -> 26.70ns (+15%) > > + div.className: 31.10ns -> 26.40ns (+18%) > > + div.nodeName: 37.70ns -> 33.00ns (+14%) > > + text.nodeValue: 31.40ns -> 25.90ns (+21%) > > + text.textContent: 51.50ns -> 45.90ns (+12%) > > Should we add it as a performance test? Bindings/dom-attributes.html should cover it. I'll update it later. (Currently, the div.id test in dom-attributes.html returns null, which is a different call-path from the performance test attached in this bug.)
WebKit Review Bot
Comment 9 2012-04-17 13:05:40 PDT
Comment on attachment 137502 [details] Patch Clearing flags on attachment: 137502 Committed r114421: <http://trac.webkit.org/changeset/114421>
WebKit Review Bot
Comment 10 2012-04-17 13:05:45 PDT
All reviewed patches have been landed. Closing bug.
Kentaro Hara
Comment 11 2012-04-19 10:00:34 PDT
Reverted r114421 for reason: Chromium crash Committed r114645: <http://trac.webkit.org/changeset/114645>
Kentaro Hara
Comment 12 2012-04-23 09:54:45 PDT
Created attachment 138368 [details] Patch for relanding. It is revealed that the patch was innocent.
Kentaro Hara
Comment 13 2012-04-23 09:55:28 PDT
Committed r114912: <http://trac.webkit.org/changeset/114912>
Note You need to log in before you can comment on or make changes to this bug.