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-

Description Geoffrey Garen 2013-02-16 16:21:59 PST
Code cache should be explicit about what it caches
Comment 1 Geoffrey Garen 2013-02-16 16:38:22 PST
Created attachment 188741 [details]
Patch
Comment 2 Geoffrey Garen 2013-02-16 16:39:16 PST
No regression on PLT.
Comment 3 Oliver Hunt 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.
Comment 4 Build Bot 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
Comment 5 Geoffrey Garen 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.
Comment 6 Geoffrey Garen 2013-02-17 11:17:02 PST
Committed r143133: <http://trac.webkit.org/changeset/143133>
Comment 7 Roger Fong 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?
Comment 8 Roger Fong 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).