Bug 110039

Summary: Code cache should be explicit about what it caches
Product: WebKit Reporter: Geoffrey Garen <ggaren>
Component: New BugsAssignee: Geoffrey Garen <ggaren>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, ddkilzer, oliver, rniwa, roger_fong, yongjun_zhang
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch oliver: review+, buildbot: commit-queue-

Geoffrey Garen
Reported 2013-02-16 16:21:59 PST
Code cache should be explicit about what it caches
Attachments
Patch (16.75 KB, patch)
2013-02-16 16:38 PST, Geoffrey Garen 		oliver: review+
buildbot: commit-queue-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Geoffrey Garen
Comment 1 2013-02-16 16:38:22 PST
Created attachment 188741 [details] Patch
Geoffrey Garen
Comment 2 2013-02-16 16:39:16 PST
No regression on PLT.
Oliver Hunt
Comment 3 2013-02-16 19:55:14 PST
Comment on attachment 188741 [details] Patch What does this do to memory usage? It looks fine, but i recall membuster disliking the strong linkage between entries in the code cache and individual function bodies.
Build Bot
Comment 4 2013-02-16 22:30:40 PST
Comment on attachment 188741 [details] Patch Attachment 188741 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/16593803 New failing tests: media/video-controls-captions-trackmenu.html
Geoffrey Garen
Comment 5 2013-02-17 09:33:37 PST
> What does this do to memory usage? It looks fine, but i recall membuster disliking the strong linkage between entries in the code cache and individual function bodies. Because a low memory warning evicts the cache, I don't think Membuster will be affected / can be used to measure. Instead, I tuned using the PLT: this character count is what you need to get an equal PLT score to the old entry count. Based on that, and since we know that a larger cache makes the PLT faster, I think memory use should be equal.
Geoffrey Garen
Comment 6 2013-02-17 11:17:02 PST
Committed r143133: <http://trac.webkit.org/changeset/143133>
Roger Fong
Comment 7 2013-02-17 18:56:44 PST
This broke the windows build: http://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/44385/steps/compile-webkit/logs/stdio The problem line is "typedef type name MapType::iterator iterator". VS dislikes the use of "typename" anywhere outside a template class declaration. Is using "typename" required here if all that's happening is a typedef of MapType::iterator?
Roger Fong
Comment 8 2013-02-17 19:36:27 PST
(In reply to comment #7) > This broke the windows build: http://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/44385/steps/compile-webkit/logs/stdio > > The problem line is "typedef type name MapType::iterator iterator". > VS dislikes the use of "typename" anywhere outside a template class declaration. Is using "typename" required here if all that's happening is a typedef of MapType::iterator? I landed a build fix here: http://trac.webkit.org/changeset/143141 to get bots happy. Let me know if I'm wrong and just blew up all of JSC (or just a little part of it).
Note You need to log in before you can comment on or make changes to this bug.