Bug 108660 – Merged the global function cache into the source code cache

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Geoffrey Garen]

Merged the global function cache into the source code cache

[Geoffrey Garen]

Created attachment 186085 [details]
Patch

[Geoffrey Garen]

Created attachment 186086 [details]
benchmark results

[Oliver Hunt]

Could you test on http://jsbench.cs.purdue.edu as well? and the plt?

[Geoffrey Garen]

> Could you test on http://jsbench.cs.purdue.edu as well?

I did. Same result.

[Geoffrey Garen]

> and the plt?

Had to bump the limit to 1280 to make the plt happy. Seems to confirm the need for an adaptive size.

[Geoffrey Garen]

> > and the plt?

(Merging the caches and keeping the net limit of 2048 is a 4% PLT speedup.)

[Geoffrey Garen]

Committed r142966: <http://trac.webkit.org/changeset/142966>

[Darin Adler]

Comment on attachment 186085 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=186085&action=review

> Source/JavaScriptCore/runtime/CodeCache.h:102
> +        , m_name(WTF::HashTableDeletedValue)

I think this not helpful. Instead m_name should be allowed to initialize itself to the null string. m_sourceString is the field that holds the deleted state and m_name need not do anything special. In fact, if there was an easy way to do so, I would suggest leaving this in an uninitialized state.

> Source/JavaScriptCore/runtime/CodeCache.h:103
> +        , m_flags(0)

Why not omit this? Why generate the extra code to zero out the m_flags field?

> Source/JavaScriptCore/runtime/CodeCache.h:132
> +struct SourceCodeKeyHashTraits : WTF::SimpleClassHashTraits<SourceCodeKey> {

Surprised you have to use the WTF:: prefix here. Should we fix that in WTF by putting adding using lines to the HashTraits.h header?

[Geoffrey Garen]

> > Source/JavaScriptCore/runtime/CodeCache.h:102
> > +        , m_name(WTF::HashTableDeletedValue)
> 
> I think this not helpful. Instead m_name should be allowed to initialize itself to the null string. m_sourceString is the field that holds the deleted state and m_name need not do anything special. In fact, if there was an easy way to do so, I would suggest leaving this in an uninitialized state.
> 
> > Source/JavaScriptCore/runtime/CodeCache.h:103
> > +        , m_flags(0)
> 
> Why not omit this? Why generate the extra code to zero out the m_flags field?

Agreed on both counts. Will fix.

> > Source/JavaScriptCore/runtime/CodeCache.h:132
> > +struct SourceCodeKeyHashTraits : WTF::SimpleClassHashTraits<SourceCodeKey> {
> 
> Surprised you have to use the WTF:: prefix here. Should we fix that in WTF by putting adding using lines to the HashTraits.h header?

Sure.

[Geoffrey Garen]

Committed r143027: <http://trac.webkit.org/changeset/143027>