Changeset 198778 in webkit for trunk/Source/JavaScriptCore/bytecode/EvalCodeCache.h

Timestamp:

Mar 29, 2016, 3:25:44 AM (9 years ago)

Author:

Yusuke Suzuki

Message:

REGRESSION(r192914): 10% regression on Sunspider's date-format-tofte
​https://bugs.webkit.org/show_bug.cgi?id=155559

Reviewed by Saam Barati.

Source/JavaScriptCore:

The fast path of the eval function is the super hot path in date-format-tofte.
Any performance regression is not allowed here.
Before this patch, we allocated SourceCode in the fast path.
This allocation incurs 10% performance regression.

This patch removes this allocation in the fast path.
And change the key of the EvalCodeCache to EvalCodeCache::CacheKey.
It combines RefPtr<StringImpl> and isArrowFunctionContext.
Since EvalCodeCache does not cache any eval code evaluated under the strict mode,
it is unnecessary to include several options (ThisTDZMode, and DerivedContextType) in the cache map's key.
But isArrowFunctionContext is necessary since the sloppy mode arrow function exists.

To validate this change, we add a new test that evaluates the same code
under the non-arrow function context and the arrow function context.

After introducing CacheKey, we observed 1% regression compared to the RefPtr<StringImpl> keyed case.
This is because HashMap<RefPtr<T>, ...>::get(T*) is specially optimized; this path is inlined while the normal ::get() is not inlined.
To avoid this performance regression, we introduce HashMap::fastGet, that aggressively encourages inlining.
The relationship between fastGet() and get() is similar to fastAdd() and add().
After applying this change, the evaluation shows no performance regression in comparison with the RefPtr<StringImpl> keyed case.

• bytecode/EvalCodeCache.h:

(JSC::EvalCodeCache::CacheKey::CacheKey):
(JSC::EvalCodeCache::CacheKey::hash):
(JSC::EvalCodeCache::CacheKey::isEmptyValue):
(JSC::EvalCodeCache::CacheKey::operator==):
(JSC::EvalCodeCache::CacheKey::isHashTableDeletedValue):
(JSC::EvalCodeCache::CacheKey::Hash::hash):
(JSC::EvalCodeCache::CacheKey::Hash::equal):
(JSC::EvalCodeCache::tryGet):
(JSC::EvalCodeCache::getSlow):
(JSC::EvalCodeCache::isCacheable):

• interpreter/Interpreter.cpp:

(JSC::eval):

• tests/stress/eval-in-arrow-function.js: Added.

(shouldBe):
(i):

Source/WTF:

Add HashTable::inlineLookup and HashMap::fastGet.

• wtf/HashMap.h:
• wtf/HashTable.h:

File:

• trunk/Source/JavaScriptCore/bytecode/EvalCodeCache.h (modified) (5 diffs)

trunk/Source/JavaScriptCore/bytecode/EvalCodeCache.h

@@ -35 +35 @@
 #include "Options.h"
 #include "SourceCode.h"
-#include "SourceCodeKey.h"
 #include <wtf/HashMap.h>
 #include <wtf/RefPtr.h>
@@ -46 +45 @@
     class EvalCodeCache {
     public:
-        EvalExecutable* tryGet(bool inStrictContext, const SourceCode& evalSource, ThisTDZMode thisTDZMode, JSScope* scope)
+        class CacheKey {
+        public:
+            CacheKey(const String& source, bool isArrowFunctionContext)
+                : m_source(source.impl())
+                , m_isArrowFunctionContext(isArrowFunctionContext)
+            {
+            }
+
+            CacheKey(WTF::HashTableDeletedValueType)
+                : m_source(WTF::HashTableDeletedValue)
+            {
+            }
+
+            CacheKey() = default;
+
+            unsigned hash() const { return m_source->hash(); }
+
+            bool isEmptyValue() const { return !m_source; }
+
+            bool operator==(const CacheKey& other) const
+            {
+                return m_source == other.m_source && m_isArrowFunctionContext == other.m_isArrowFunctionContext;
+            }
+
+            bool isHashTableDeletedValue() const { return m_source.isHashTableDeletedValue(); }
+
+            struct Hash {
+                static unsigned hash(const CacheKey& key)
+                {
+                    return key.hash();
+                }
+                static bool equal(const CacheKey& lhs, const CacheKey& rhs)
+                {
+                    return StringHash::equal(lhs.m_source, rhs.m_source) && lhs.m_isArrowFunctionContext == rhs.m_isArrowFunctionContext;
+                }
+                static const bool safeToCompareToEmptyOrDeleted = false;
+            };
+
+            typedef SimpleClassHashTraits<CacheKey> HashTraits;
+
+        private:
+            RefPtr<StringImpl> m_source;
+            bool m_isArrowFunctionContext { false };
+        };
+
+        EvalExecutable* tryGet(bool inStrictContext, const String& evalSource, bool isArrowFunctionContext, JSScope* scope)
         {
             if (isCacheable(inStrictContext, evalSource, scope)) {
                 ASSERT(!inStrictContext);
-                SourceCodeKey sourceCodeKey(evalSource, String(), SourceCodeKey::EvalType, JSParserBuiltinMode::NotBuiltin, JSParserStrictMode::NotStrict, thisTDZMode);
-                return m_cacheMap.get(sourceCodeKey).get();
+                return m_cacheMap.fastGet(CacheKey(evalSource, isArrowFunctionContext)).get();
             }
             return nullptr;
         }
         
-        EvalExecutable* getSlow(ExecState* exec, JSCell* owner, bool inStrictContext, ThisTDZMode thisTDZMode, DerivedContextType derivedContextType, bool isArrowFunctionContext, const SourceCode& evalSource, JSScope* scope)
+        EvalExecutable* getSlow(ExecState* exec, JSCell* owner, bool inStrictContext, ThisTDZMode thisTDZMode, DerivedContextType derivedContextType, bool isArrowFunctionContext, const String& evalSource, JSScope* scope)
         {
             VariableEnvironment variablesUnderTDZ;
             JSScope::collectVariablesUnderTDZ(scope, variablesUnderTDZ);
-            EvalExecutable* evalExecutable = EvalExecutable::create(exec, evalSource, inStrictContext, thisTDZMode, derivedContextType, isArrowFunctionContext, &variablesUnderTDZ);
-
+            EvalExecutable* evalExecutable = EvalExecutable::create(exec, makeSource(evalSource), inStrictContext, thisTDZMode, derivedContextType, isArrowFunctionContext, &variablesUnderTDZ);
             if (!evalExecutable)
                 return nullptr;
@@ -67 +109 @@
             if (isCacheable(inStrictContext, evalSource, scope) && m_cacheMap.size() < maxCacheEntries) {
                 ASSERT(!inStrictContext);
-                SourceCodeKey sourceCodeKey(evalSource, String(), SourceCodeKey::EvalType, JSParserBuiltinMode::NotBuiltin, JSParserStrictMode::NotStrict, thisTDZMode);
-                m_cacheMap.set(sourceCodeKey, WriteBarrier<EvalExecutable>(exec->vm(), owner, evalExecutable));
+                ASSERT_WITH_MESSAGE(thisTDZMode == ThisTDZMode::CheckIfNeeded, "Always CheckIfNeeded because the caching is enabled only in the sloppy mode.");
+                ASSERT_WITH_MESSAGE(derivedContextType == DerivedContextType::None, "derivedContextType is always None because class methods and class constructors are always evaluated as the strict code.");
+                m_cacheMap.set(CacheKey(evalSource, isArrowFunctionContext), WriteBarrier<EvalExecutable>(exec->vm(), owner, evalExecutable));
             }
             
@@ -89 +132 @@
         }
 
-        ALWAYS_INLINE bool isCacheable(bool inStrictContext, const SourceCode& evalSource, JSScope* scope)
+        ALWAYS_INLINE bool isCacheable(bool inStrictContext, const String& evalSource, JSScope* scope)
         {
             // If eval() is called and it has access to a lexical scope, we can't soundly cache it.
@@ -99 +142 @@
         static const int maxCacheEntries = 64;
 
-        typedef HashMap<SourceCodeKey, WriteBarrier<EvalExecutable>, SourceCodeKeyHash, SourceCodeKeyHashTraits> EvalCacheMap;
+        typedef HashMap<CacheKey, WriteBarrier<EvalExecutable>, CacheKey::Hash, CacheKey::HashTraits> EvalCacheMap;
         EvalCacheMap m_cacheMap;
     };