[webkit-changes] [201235] trunk/Source/JavaScriptCore

Fri, 20 May 2016 17:17:39 -0700

Title: [201235] trunk/Source/_javascript_Core
Revision
201235
Author
[email protected]
Date
2016-05-20 17:17:30 -0700 (Fri, 20 May 2016)

Log Message

JSScope::abstractAccess doesn't need to copy the SymbolTableEntry, it can use it by reference
https://bugs.webkit.org/show_bug.cgi?id=157956

Reviewed by Geoffrey Garen.

A SymbolTableEntry may be a FatEntry. Copying a FatEntry is slow because we have to
malloc memory for it, then free the malloced memory once the entry goes out of
scope. abstractAccess uses a SymbolTableEntry temporarily when performing scope
accesses during bytecode linking. It copies out the SymbolTableEntry every time
it does a SymbolTable lookup. This is not cheap when the entry happens to be a
FatEntry. We should really just be using a reference to the entry because
there is no need to copy it in such a scenario.

* runtime/JSScope.cpp:
(JSC::abstractAccess):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (201234 => 201235)


--- trunk/Source/_javascript_Core/ChangeLog	2016-05-20 23:56:34 UTC (rev 201234)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-05-21 00:17:30 UTC (rev 201235)
@@ -1,3 +1,21 @@
+2016-05-20  Saam barati  <[email protected]>
+
+        JSScope::abstractAccess doesn't need to copy the SymbolTableEntry, it can use it by reference
+        https://bugs.webkit.org/show_bug.cgi?id=157956
+
+        Reviewed by Geoffrey Garen.
+
+        A SymbolTableEntry may be a FatEntry. Copying a FatEntry is slow because we have to
+        malloc memory for it, then free the malloced memory once the entry goes out of
+        scope. abstractAccess uses a SymbolTableEntry temporarily when performing scope
+        accesses during bytecode linking. It copies out the SymbolTableEntry every time
+        it does a SymbolTable lookup. This is not cheap when the entry happens to be a
+        FatEntry. We should really just be using a reference to the entry because
+        there is no need to copy it in such a scenario.
+
+        * runtime/JSScope.cpp:
+        (JSC::abstractAccess):
+
 2016-05-20  Joseph Pecoraro  <[email protected]>
 
         Web Inspector: retained size for typed arrays does not count native backing store

Modified: trunk/Source/_javascript_Core/runtime/JSScope.cpp (201234 => 201235)


--- trunk/Source/_javascript_Core/runtime/JSScope.cpp	2016-05-20 23:56:34 UTC (rev 201234)
+++ trunk/Source/_javascript_Core/runtime/JSScope.cpp	2016-05-21 00:17:30 UTC (rev 201235)
@@ -56,19 +56,21 @@
             op = ResolveOp(Dynamic, 0, 0, 0, 0, 0);
             return true;
         }
+        SymbolTable* symbolTable = lexicalEnvironment->symbolTable();
+        ConcurrentJITLocker locker(symbolTable->m_lock);
+        auto iter = symbolTable->find(locker, ident.impl());
+        if (iter != symbolTable->end(locker)) {
+            SymbolTableEntry& entry = iter->value;
+            ASSERT(!entry.isNull());
+            if (entry.isReadOnly() && getOrPut == Put) {
+                // We know the property will be at this lexical environment scope, but we don't know how to cache it.
+                op = ResolveOp(Dynamic, 0, 0, 0, 0, 0);
+                return true;
+            }
 
-        SymbolTableEntry entry = lexicalEnvironment->symbolTable()->get(ident.impl());
-        if (entry.isReadOnly() && getOrPut == Put) {
-            // We know the property will be at this lexical environment scope, but we don't know how to cache it.
-            op = ResolveOp(Dynamic, 0, 0, 0, 0, 0);
-            return true;
-        }
-
-        if (!entry.isNull()) {
             op = ResolveOp(makeType(ClosureVar, needsVarInjectionChecks), depth, 0, lexicalEnvironment, entry.watchpointSet(), entry.scopeOffset().offset());
             return true;
         }
-
         if (scope->type() == ModuleEnvironmentType) {
             JSModuleEnvironment* moduleEnvironment = jsCast<JSModuleEnvironment*>(scope);
             JSModuleRecord* moduleRecord = moduleEnvironment->moduleRecord();
@@ -76,22 +78,30 @@
             if (resolution.type == JSModuleRecord::Resolution::Type::Resolved) {
                 JSModuleRecord* importedRecord = resolution.moduleRecord;
                 JSModuleEnvironment* importedEnvironment = importedRecord->moduleEnvironment();
-                SymbolTableEntry entry = importedEnvironment->symbolTable()->get(resolution.localName.impl());
+                SymbolTable* symbolTable = importedEnvironment->symbolTable();
+                ConcurrentJITLocker locker(symbolTable->m_lock);
+                auto iter = symbolTable->find(locker, resolution.localName.impl());
+                ASSERT(iter != symbolTable->end(locker));
+                SymbolTableEntry& entry = iter->value;
                 ASSERT(!entry.isNull());
                 op = ResolveOp(makeType(ModuleVar, needsVarInjectionChecks), depth, 0, importedEnvironment, entry.watchpointSet(), entry.scopeOffset().offset(), resolution.localName.impl());
                 return true;
             }
         }
 
-        if (lexicalEnvironment->symbolTable()->usesNonStrictEval())
+        if (symbolTable->usesNonStrictEval())
             needsVarInjectionChecks = true;
         return false;
     }
 
     if (scope->isGlobalLexicalEnvironment()) {
         JSGlobalLexicalEnvironment* globalLexicalEnvironment = jsCast<JSGlobalLexicalEnvironment*>(scope);
-        SymbolTableEntry entry = globalLexicalEnvironment->symbolTable()->get(ident.impl());
-        if (!entry.isNull()) {
+        SymbolTable* symbolTable = globalLexicalEnvironment->symbolTable();
+        ConcurrentJITLocker locker(symbolTable->m_lock);
+        auto iter = symbolTable->find(locker, ident.impl());
+        if (iter != symbolTable->end(locker)) {
+            SymbolTableEntry& entry = iter->value;
+            ASSERT(!entry.isNull());
             if (getOrPut == Put && entry.isReadOnly() && !isInitialization(initializationMode)) {
                 // We know the property will be at global lexical environment, but we don't know how to cache it.
                 op = ResolveOp(Dynamic, 0, 0, 0, 0, 0);
@@ -118,18 +128,24 @@
 
     if (scope->isGlobalObject()) {
         JSGlobalObject* globalObject = jsCast<JSGlobalObject*>(scope);
-        SymbolTableEntry entry = globalObject->symbolTable()->get(ident.impl());
-        if (!entry.isNull()) {
-            if (getOrPut == Put && entry.isReadOnly()) {
-                // We know the property will be at global scope, but we don't know how to cache it.
-                op = ResolveOp(Dynamic, 0, 0, 0, 0, 0);
+        {
+            SymbolTable* symbolTable = globalObject->symbolTable();
+            ConcurrentJITLocker locker(symbolTable->m_lock);
+            auto iter = symbolTable->find(locker, ident.impl());
+            if (iter != symbolTable->end(locker)) {
+                SymbolTableEntry& entry = iter->value;
+                ASSERT(!entry.isNull());
+                if (getOrPut == Put && entry.isReadOnly()) {
+                    // We know the property will be at global scope, but we don't know how to cache it.
+                    op = ResolveOp(Dynamic, 0, 0, 0, 0, 0);
+                    return true;
+                }
+
+                op = ResolveOp(
+                    makeType(GlobalVar, needsVarInjectionChecks), depth, 0, 0, entry.watchpointSet(),
+                    reinterpret_cast<uintptr_t>(globalObject->variableAt(entry.scopeOffset()).slot()));
                 return true;
             }
-
-            op = ResolveOp(
-                makeType(GlobalVar, needsVarInjectionChecks), depth, 0, 0, entry.watchpointSet(),
-                reinterpret_cast<uintptr_t>(globalObject->variableAt(entry.scopeOffset()).slot()));
-            return true;
         }
 
         PropertySlot slot(globalObject, PropertySlot::InternalMethodType::VMInquiry);

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to