Remove some unnecessary RefPtrs in the parser
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Apr 2016 23:30:36 +0000 (23:30 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Apr 2016 23:30:36 +0000 (23:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=156865

Reviewed by Filip Pizlo.

The IdentifierArena or the SourceProviderCacheItem will own these UniquedStringImpls
while we are using them. There is no need for us to reference count them.

This might be a 0.5% speedup on octane code-load.

* parser/Parser.cpp:
(JSC::Parser<LexerType>::parseInner):
* parser/Parser.h:
(JSC::Scope::setIsLexicalScope):
(JSC::Scope::isLexicalScope):
(JSC::Scope::closedVariableCandidates):
(JSC::Scope::declaredVariables):
(JSC::Scope::lexicalVariables):
(JSC::Scope::finalizeLexicalEnvironment):
(JSC::Scope::computeLexicallyCapturedVariablesAndPurgeCandidates):
(JSC::Scope::collectFreeVariables):
(JSC::Scope::getCapturedVars):
(JSC::Scope::setStrictMode):
(JSC::Scope::isValidStrictMode):
(JSC::Scope::shadowsArguments):
(JSC::Scope::copyCapturedVariablesToVector):
* parser/SourceProviderCacheItem.h:
(JSC::SourceProviderCacheItem::usedVariables):
(JSC::SourceProviderCacheItem::~SourceProviderCacheItem):
(JSC::SourceProviderCacheItem::create):
(JSC::SourceProviderCacheItem::SourceProviderCacheItem):
(JSC::SourceProviderCacheItem::writtenVariables): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@199845 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/parser/Parser.cpp
Source/JavaScriptCore/parser/Parser.h
Source/JavaScriptCore/parser/SourceProviderCacheItem.h

index 6cf6ef2..095bba1 100644 (file)
@@ -1,3 +1,38 @@
+2016-04-21  Saam barati  <sbarati@apple.com>
+
+        Remove some unnecessary RefPtrs in the parser
+        https://bugs.webkit.org/show_bug.cgi?id=156865
+
+        Reviewed by Filip Pizlo.
+
+        The IdentifierArena or the SourceProviderCacheItem will own these UniquedStringImpls
+        while we are using them. There is no need for us to reference count them.
+
+        This might be a 0.5% speedup on octane code-load.
+
+        * parser/Parser.cpp:
+        (JSC::Parser<LexerType>::parseInner):
+        * parser/Parser.h:
+        (JSC::Scope::setIsLexicalScope):
+        (JSC::Scope::isLexicalScope):
+        (JSC::Scope::closedVariableCandidates):
+        (JSC::Scope::declaredVariables):
+        (JSC::Scope::lexicalVariables):
+        (JSC::Scope::finalizeLexicalEnvironment):
+        (JSC::Scope::computeLexicallyCapturedVariablesAndPurgeCandidates):
+        (JSC::Scope::collectFreeVariables):
+        (JSC::Scope::getCapturedVars):
+        (JSC::Scope::setStrictMode):
+        (JSC::Scope::isValidStrictMode):
+        (JSC::Scope::shadowsArguments):
+        (JSC::Scope::copyCapturedVariablesToVector):
+        * parser/SourceProviderCacheItem.h:
+        (JSC::SourceProviderCacheItem::usedVariables):
+        (JSC::SourceProviderCacheItem::~SourceProviderCacheItem):
+        (JSC::SourceProviderCacheItem::create):
+        (JSC::SourceProviderCacheItem::SourceProviderCacheItem):
+        (JSC::SourceProviderCacheItem::writtenVariables): Deleted.
+
 2016-04-21  Filip Pizlo  <fpizlo@apple.com>
 
         PolymorphicAccess adds sizeof(CallerFrameAndPC) rather than subtracting it when calculating stack height
index 4715bc5..28bbe53 100644 (file)
@@ -318,10 +318,10 @@ String Parser<LexerType>::parseInner(const Identifier& calleeName, SourceParseMo
 #ifndef NDEBUG
     if (m_parsingBuiltin && isProgramParseMode(parseMode)) {
         VariableEnvironment& lexicalVariables = scope->lexicalVariables();
-        const IdentifierSet& closedVariableCandidates = scope->closedVariableCandidates();
+        const HashSet<UniquedStringImpl*>& closedVariableCandidates = scope->closedVariableCandidates();
         const BuiltinNames& builtinNames = m_vm->propertyNames->builtinNames();
-        for (const RefPtr<UniquedStringImpl>& candidate : closedVariableCandidates) {
-            if (!lexicalVariables.contains(candidate) && !varDeclarations.contains(candidate) && !builtinNames.isPrivateName(*candidate.get())) {
+        for (UniquedStringImpl* candidate : closedVariableCandidates) {
+            if (!lexicalVariables.contains(candidate) && !varDeclarations.contains(candidate) && !builtinNames.isPrivateName(*candidate)) {
                 dataLog("Bad global capture in builtin: '", candidate, "'\n");
                 dataLog(m_source->view());
                 CRASH();
index 5113f05..0c26c8b 100644 (file)
@@ -297,7 +297,7 @@ public:
     }
     bool isLexicalScope() { return m_isLexicalScope; }
 
-    const IdentifierSet& closedVariableCandidates() const { return m_closedVariableCandidates; }
+    const HashSet<UniquedStringImpl*>& closedVariableCandidates() const { return m_closedVariableCandidates; }
     VariableEnvironment& declaredVariables() { return m_declaredVariables; }
     VariableEnvironment& lexicalVariables() { return m_lexicalVariables; }
     VariableEnvironment& finalizeLexicalEnvironment() 
@@ -323,16 +323,15 @@ public:
         // lexical scope off the stack, we should find which variables are truly captured, and which
         // variable still may be captured in a parent scope.
         if (m_lexicalVariables.size() && m_closedVariableCandidates.size()) {
-            auto end = m_closedVariableCandidates.end();
-            for (auto iter = m_closedVariableCandidates.begin(); iter != end; ++iter)
-                m_lexicalVariables.markVariableAsCapturedIfDefined(iter->get());
+            for (UniquedStringImpl* impl : m_closedVariableCandidates)
+                m_lexicalVariables.markVariableAsCapturedIfDefined(impl);
         }
 
         // We can now purge values from the captured candidates because they're captured in this scope.
         {
             for (auto entry : m_lexicalVariables) {
                 if (entry.value.isCaptured())
-                    m_closedVariableCandidates.remove(entry.key);
+                    m_closedVariableCandidates.remove(entry.key.get());
             }
         }
     }
@@ -587,8 +586,8 @@ public:
         // Propagate closed variable candidates downwards within the same function.
         // Cross function captures will be realized via m_usedVariables propagation.
         if (shouldTrackClosedVariables && !nestedScope->m_isFunctionBoundary && nestedScope->m_closedVariableCandidates.size()) {
-            IdentifierSet::iterator end = nestedScope->m_closedVariableCandidates.end();
-            IdentifierSet::iterator begin = nestedScope->m_closedVariableCandidates.begin();
+            auto end = nestedScope->m_closedVariableCandidates.end();
+            auto begin = nestedScope->m_closedVariableCandidates.begin();
             m_closedVariableCandidates.add(begin, end);
         }
     }
@@ -624,11 +623,11 @@ public:
                 capturedVariables.add(entry.key);
             return;
         }
-        for (IdentifierSet::iterator ptr = m_closedVariableCandidates.begin(); ptr != m_closedVariableCandidates.end(); ++ptr) {
+        for (UniquedStringImpl* impl : m_closedVariableCandidates) {
             // We refer to m_declaredVariables here directly instead of a hasDeclaredVariable because we want to mark the callee as captured.
-            if (!m_declaredVariables.contains(*ptr)) 
+            if (!m_declaredVariables.contains(impl)) 
                 continue;
-            capturedVariables.add(*ptr);
+            capturedVariables.add(impl);
         }
     }
     void setStrictMode() { m_strictMode = true; }
@@ -636,9 +635,9 @@ public:
     bool isValidStrictMode() const { return m_isValidStrictMode; }
     bool shadowsArguments() const { return m_shadowsArguments; }
 
-    void copyCapturedVariablesToVector(const UniquedStringImplPtrSet& capturedVariables, Vector<RefPtr<UniquedStringImpl>>& vector)
+    void copyCapturedVariablesToVector(const UniquedStringImplPtrSet& usedVariables, Vector<UniquedStringImpl*, 8>& vector)
     {
-        for (UniquedStringImpl* impl : capturedVariables) {
+        for (UniquedStringImpl* impl : usedVariables) {
             if (m_declaredVariables.contains(impl) || m_lexicalVariables.contains(impl))
                 continue;
             vector.append(impl);
@@ -740,7 +739,7 @@ private:
     VariableEnvironment m_lexicalVariables;
     Vector<UniquedStringImplPtrSet, 6> m_usedVariables;
     UniquedStringImplPtrSet m_sloppyModeHoistableFunctionCandidates;
-    IdentifierSet m_closedVariableCandidates;
+    HashSet<UniquedStringImpl*> m_closedVariableCandidates;
     RefPtr<ModuleScopeData> m_moduleScopeData;
     DeclarationStacks::FunctionStack m_functionDeclarations;
 };
index dd25521..89c82a1 100644 (file)
@@ -45,8 +45,7 @@ struct SourceProviderCacheItemCreationParameters {
     bool usesEval;
     bool strictMode;
     InnerArrowFunctionCodeFeatures innerArrowFunctionFeatures;
-    Vector<RefPtr<UniquedStringImpl>> usedVariables;
-    Vector<RefPtr<UniquedStringImpl>> writtenVariables;
+    Vector<UniquedStringImpl*, 8> usedVariables;
     bool isBodyArrowExpression { false };
     JSTokenType tokenType { CLOSEBRACE };
 };
@@ -93,10 +92,8 @@ public:
 
     unsigned lastTockenLineStartOffset;
     unsigned usedVariablesCount;
-    unsigned writtenVariablesCount;
 
     UniquedStringImpl** usedVariables() const { return const_cast<UniquedStringImpl**>(m_variables); }
-    UniquedStringImpl** writtenVariables() const { return const_cast<UniquedStringImpl**>(&m_variables[usedVariablesCount]); }
     bool isBodyArrowExpression;
     JSTokenType tokenType;
 
@@ -108,13 +105,13 @@ private:
 
 inline SourceProviderCacheItem::~SourceProviderCacheItem()
 {
-    for (unsigned i = 0; i < usedVariablesCount + writtenVariablesCount; ++i)
+    for (unsigned i = 0; i < usedVariablesCount; ++i)
         m_variables[i]->deref();
 }
 
 inline std::unique_ptr<SourceProviderCacheItem> SourceProviderCacheItem::create(const SourceProviderCacheItemCreationParameters& parameters)
 {
-    size_t variableCount = parameters.writtenVariables.size() + parameters.usedVariables.size();
+    size_t variableCount = parameters.usedVariables.size();
     size_t objectSize = sizeof(SourceProviderCacheItem) + sizeof(UniquedStringImpl*) * variableCount;
     void* slot = fastMalloc(objectSize);
     return std::unique_ptr<SourceProviderCacheItem>(new (slot) SourceProviderCacheItem(parameters));
@@ -133,18 +130,12 @@ inline SourceProviderCacheItem::SourceProviderCacheItem(const SourceProviderCach
     , innerArrowFunctionFeatures(parameters.innerArrowFunctionFeatures)
     , lastTockenLineStartOffset(parameters.lastTockenLineStartOffset)
     , usedVariablesCount(parameters.usedVariables.size())
-    , writtenVariablesCount(parameters.writtenVariables.size())
     , isBodyArrowExpression(parameters.isBodyArrowExpression)
     , tokenType(parameters.tokenType)
 {
-    unsigned j = 0;
-    for (unsigned i = 0; i < usedVariablesCount; ++i, ++j) {
-        m_variables[j] = parameters.usedVariables[i].get();
-        m_variables[j]->ref();
-    }
-    for (unsigned i = 0; i < writtenVariablesCount; ++i, ++j) {
-        m_variables[j] = parameters.writtenVariables[i].get();
-        m_variables[j]->ref();
+    for (unsigned i = 0; i < usedVariablesCount; ++i) {
+        m_variables[i] = parameters.usedVariables[i];
+        m_variables[i]->ref();
     }
 }