Summary: | Save space on keys in the CodeCache | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Geoffrey Garen <ggaren> | ||||||
Component: | New Bugs | Assignee: | Geoffrey Garen <ggaren> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | kling, oliver, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 110242 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Geoffrey Garen
2013-02-18 22:41:31 PST
Created attachment 188996 [details]
Patch
Attachment 188996 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/runtime/CodeCache.cpp', u'Source/JavaScriptCore/runtime/CodeCache.h']" exit_code: 1
Source/JavaScriptCore/runtime/CodeCache.h:60: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4]
Source/JavaScriptCore/runtime/CodeCache.h:61: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4]
Total errors found: 2 in 3 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 188996 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=188996&action=review > Source/JavaScriptCore/runtime/CodeCache.h:86 > + size_t length() const { return string().length(); } Wouldn't it be cheaper to call m_sourceCode.length() here? > > Source/JavaScriptCore/runtime/CodeCache.h:86
> > + size_t length() const { return string().length(); }
>
> Wouldn't it be cheaper to call m_sourceCode.length() here?
It would! Will fix.
Comment on attachment 188996 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=188996&action=review > Source/JavaScriptCore/runtime/CodeCache.h:58 > - enum CodeType { EvalType, ProgramType, FunctionCallType, FunctionConstructType }; > + enum CodeType { EvalType, ProgramType, FunctionType }; Where is FunctionType used? I'm not sure where the CodeCache can directly interact with Functions now, and a brief looksy doesn't seem to imply that it does. The FunctionType enum should be removed as well. Created attachment 189104 [details]
Patch
> Where is FunctionType used?
FunctionType is used by 'new Function', which is still cached. (See getFunctionExecutableFromGlobalCode.)
Incorporated Andreas's fix, which in turn showed some more opportunities for simplifying based on m_sourceCode. Comment on attachment 189104 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189104&action=review > Source/JavaScriptCore/runtime/CodeCache.h:94 > - && m_sourceString == other.m_sourceString; > + && string() == other.string(); You could add a length check here as well, to further minimize the chance of having to incorrectly reify the string > You could add a length check here as well, to further minimize the chance of having to incorrectly reify the string
Nice! I didn't do that before because length() was expensive; but now it's inexpensive, so I will.
Committed r143348: <http://trac.webkit.org/changeset/143348> Re-opened since this is blocked by bug 110242 Re-landed in <http://trac.webkit.org/changeset/143384>. |