Bug 110039 - Code cache should be explicit about what it caches
Summary: Code cache should be explicit about what it caches
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-16 16:21 PST by Geoffrey Garen
Modified: 2013-02-17 19:36 PST (History)
6 users (show)

See Also:


Attachments
Patch (16.75 KB, patch)
2013-02-16 16:38 PST, Geoffrey Garen
oliver: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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).