Attachment 340444 Details for Bug 185637 – patch

[patch] patch

c-backup.diff (text/plain), 15.43 KB, created by Saam Barati on 2018-05-15 16:35:02 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2018-05-15 16:35:02 PDT

Size: 15.43 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 231819)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,33 @@
>+2018-05-15  Saam Barati  <sbarati@apple.com>
>+
>+        UnlinkedFunctionExecutable doesn't need a parent source override field since it's only used for default class constructors
>+        https://bugs.webkit.org/show_bug.cgi?id=185637
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We had this general mechanism for overriding an UnlinkedFunctionExecutable's parent
>+        source code. However, we were only using this for default class constructors. There
>+        are only two types of default class constructors. This patch makes it so that
>+        we just store this information inside of a single bit, and ask for the source
>+        code as needed instead of holding it in a nullable field that is 24 bytes in size.
>+        
>+        This brings UnlinkedFunctionExecutable's size down from 184 bytes to 160 bytes.
>+        This has the consequence of making it allocated out of a 160 byte size class
>+        instead of a 224 byte size class. This should bring down its memory footprint
>+        by ~40%.
>+
>+        * builtins/BuiltinExecutables.cpp:
>+        (JSC::BuiltinExecutables::defaultConstructorSourceCode):
>+        (JSC::BuiltinExecutables::createDefaultConstructor):
>+        (JSC::BuiltinExecutables::createExecutable):
>+        * builtins/BuiltinExecutables.h:
>+        * bytecode/UnlinkedFunctionExecutable.cpp:
>+        (JSC::UnlinkedFunctionExecutable::UnlinkedFunctionExecutable):
>+        (JSC::UnlinkedFunctionExecutable::link):
>+        * bytecode/UnlinkedFunctionExecutable.h:
>+        * runtime/CodeCache.cpp:
>+        (JSC::CodeCache::getUnlinkedGlobalFunctionExecutable):
>+
> 2018-05-15  Devin Rousso  <webkit@devinrousso.com>
> 
>         Web Inspector: Add rulers and guides
>Index: Source/JavaScriptCore/builtins/BuiltinExecutables.cpp
>===================================================================
>--- Source/JavaScriptCore/builtins/BuiltinExecutables.cpp	(revision 231813)
>+++ Source/JavaScriptCore/builtins/BuiltinExecutables.cpp	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2014-2017 Apple Inc. All rights reserved.
>+ * Copyright (C) 2014-2018 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -42,18 +42,41 @@ BuiltinExecutables::BuiltinExecutables(V
> {
> }
> 
>+const SourceCode& BuiltinExecutables::defaultConstructorSourceCode(ConstructorKind constructorKind)
>+{
>+    switch (constructorKind) {
>+    case ConstructorKind::None:
>+        RELEASE_ASSERT_NOT_REACHED();
>+    case ConstructorKind::Base: {
>+        static NeverDestroyed<const String> baseConstructorCode(MAKE_STATIC_STRING_IMPL("(function () { })"));
>+        static LazyNeverDestroyed<SourceCode> result;
>+        static std::once_flag onceFlag;
>+        std::call_once(onceFlag, [&] {
>+            result.construct(makeSource(baseConstructorCode, { }));
>+        });
>+        return result;
>+    }
>+    case ConstructorKind::Extends:
>+        static NeverDestroyed<const String> derivedConstructorCode(MAKE_STATIC_STRING_IMPL("(function (...args) { super(...args); })"));
>+        static LazyNeverDestroyed<SourceCode> result;
>+        static std::once_flag onceFlag;
>+        std::call_once(onceFlag, [&] {
>+            result.construct(makeSource(derivedConstructorCode, { }));
>+        });
>+        return result;
>+    }
>+}
>+
> UnlinkedFunctionExecutable* BuiltinExecutables::createDefaultConstructor(ConstructorKind constructorKind, const Identifier& name)
> {
>-    static NeverDestroyed<const String> baseConstructorCode(MAKE_STATIC_STRING_IMPL("(function () { })"));
>-    static NeverDestroyed<const String> derivedConstructorCode(MAKE_STATIC_STRING_IMPL("(function (...args) { super(...args); })"));
> 
>     switch (constructorKind) {
>     case ConstructorKind::None:
>         break;
>     case ConstructorKind::Base:
>-        return createExecutable(m_vm, makeSource(baseConstructorCode, { }), name, constructorKind, ConstructAbility::CanConstruct);
>+        return createExecutable(m_vm, defaultConstructorSourceCode(constructorKind), name, constructorKind, ConstructAbility::CanConstruct);
>     case ConstructorKind::Extends:
>-        return createExecutable(m_vm, makeSource(derivedConstructorCode, { }), name, constructorKind, ConstructAbility::CanConstruct);
>+        return createExecutable(m_vm, defaultConstructorSourceCode(constructorKind), name, constructorKind, ConstructAbility::CanConstruct);
>     }
>     ASSERT_NOT_REACHED();
>     return nullptr;
>@@ -73,10 +96,9 @@ UnlinkedFunctionExecutable* BuiltinExecu
> {
>     JSTextPosition positionBeforeLastNewline;
>     ParserError error;
>-    bool isParsingDefaultConstructor = constructorKind != ConstructorKind::None;
>-    JSParserBuiltinMode builtinMode = isParsingDefaultConstructor ? JSParserBuiltinMode::NotBuiltin : JSParserBuiltinMode::Builtin;
>-    UnlinkedFunctionKind kind = isParsingDefaultConstructor ? UnlinkedNormalFunction : UnlinkedBuiltinFunction;
>-    SourceCode parentSourceOverride = isParsingDefaultConstructor ? source : SourceCode();
>+    bool isBuiltinDefaultClassConstructor = constructorKind != ConstructorKind::None;
>+    JSParserBuiltinMode builtinMode = isBuiltinDefaultClassConstructor ? JSParserBuiltinMode::NotBuiltin : JSParserBuiltinMode::Builtin;
>+    UnlinkedFunctionKind kind = isBuiltinDefaultClassConstructor ? UnlinkedNormalFunction : UnlinkedBuiltinFunction;
>     std::unique_ptr<ProgramNode> program = parse<ProgramNode>(
>         &vm, source, Identifier(), builtinMode,
>         JSParserStrictMode::NotStrict, JSParserScriptMode::Classic, SourceParseMode::ProgramMode, SuperBinding::NotNeeded, error,
>@@ -105,7 +127,7 @@ UnlinkedFunctionExecutable* BuiltinExecu
>     RELEASE_ASSERT(metadata);
>     metadata->overrideName(name);
>     VariableEnvironment dummyTDZVariables;
>-    UnlinkedFunctionExecutable* functionExecutable = UnlinkedFunctionExecutable::create(&vm, source, metadata, kind, constructAbility, JSParserScriptMode::Classic, dummyTDZVariables, DerivedContextType::None, WTFMove(parentSourceOverride));
>+    UnlinkedFunctionExecutable* functionExecutable = UnlinkedFunctionExecutable::create(&vm, source, metadata, kind, constructAbility, JSParserScriptMode::Classic, dummyTDZVariables, DerivedContextType::None, isBuiltinDefaultClassConstructor);
>     return functionExecutable;
> }
> 
>Index: Source/JavaScriptCore/builtins/BuiltinExecutables.h
>===================================================================
>--- Source/JavaScriptCore/builtins/BuiltinExecutables.h	(revision 231813)
>+++ Source/JavaScriptCore/builtins/BuiltinExecutables.h	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2014 Apple Inc. All rights reserved.
>+ * Copyright (C) 2014-2018 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -49,6 +49,7 @@ const SourceCode& name##Source() { retur
>     JSC_FOREACH_BUILTIN_CODE(EXPOSE_BUILTIN_EXECUTABLES)
> #undef EXPOSE_BUILTIN_SOURCES
> 
>+    static const SourceCode& defaultConstructorSourceCode(ConstructorKind);
>     UnlinkedFunctionExecutable* createDefaultConstructor(ConstructorKind, const Identifier& name);
> 
>     static UnlinkedFunctionExecutable* createExecutable(VM&, const SourceCode&, const Identifier&, ConstructorKind, ConstructAbility);
>Index: Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.cpp
>===================================================================
>--- Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.cpp	(revision 231813)
>+++ Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.cpp	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2012-2013, 2015-2016 Apple Inc. All Rights Reserved.
>+ * Copyright (C) 2012-2018 Apple Inc. All Rights Reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -26,6 +26,7 @@
> #include "config.h"
> #include "UnlinkedFunctionExecutable.h"
> 
>+#include "BuiltinExecutables.h"
> #include "BytecodeGenerator.h"
> #include "ClassInfo.h"
> #include "CodeCache.h"
>@@ -76,7 +77,7 @@ static UnlinkedFunctionCodeBlock* genera
>     return result;
> }
> 
>-UnlinkedFunctionExecutable::UnlinkedFunctionExecutable(VM* vm, Structure* structure, const SourceCode& parentSource, SourceCode&& parentSourceOverride, FunctionMetadataNode* node, UnlinkedFunctionKind kind, ConstructAbility constructAbility, JSParserScriptMode scriptMode, VariableEnvironment& parentScopeTDZVariables, DerivedContextType derivedContextType)
>+UnlinkedFunctionExecutable::UnlinkedFunctionExecutable(VM* vm, Structure* structure, const SourceCode& parentSource, FunctionMetadataNode* node, UnlinkedFunctionKind kind, ConstructAbility constructAbility, JSParserScriptMode scriptMode, VariableEnvironment& parentScopeTDZVariables, DerivedContextType derivedContextType, bool isBuiltinDefaultClassConstructor)
>     : Base(*vm, structure)
>     , m_firstLineOffset(node->firstLine() - parentSource.firstLine().oneBasedInt())
>     , m_lineCount(node->lastLine() - node->firstLine())
>@@ -94,6 +95,7 @@ UnlinkedFunctionExecutable::UnlinkedFunc
>     , m_isInStrictContext(node->isInStrictContext())
>     , m_hasCapturedVariables(false)
>     , m_isBuiltinFunction(kind == UnlinkedBuiltinFunction)
>+    , m_isBuiltinDefaultClassConstructor(isBuiltinDefaultClassConstructor)
>     , m_constructAbility(static_cast<unsigned>(constructAbility))
>     , m_constructorKind(static_cast<unsigned>(node->constructorKind()))
>     , m_functionMode(static_cast<unsigned>(node->functionMode()))
>@@ -103,7 +105,6 @@ UnlinkedFunctionExecutable::UnlinkedFunc
>     , m_name(node->ident())
>     , m_ecmaName(node->ecmaName())
>     , m_inferredName(node->inferredName())
>-    , m_parentSourceOverride(WTFMove(parentSourceOverride))
>     , m_classSource(node->classSource())
>     , m_parentScopeTDZVariables(vm->m_compactVariableMap->get(parentScopeTDZVariables))
> {
>@@ -114,6 +115,7 @@ UnlinkedFunctionExecutable::UnlinkedFunc
>     ASSERT(m_scriptMode == static_cast<unsigned>(scriptMode));
>     ASSERT(m_superBinding == static_cast<unsigned>(node->superBinding()));
>     ASSERT(m_derivedContextType == static_cast<unsigned>(derivedContextType));
>+    ASSERT(!(m_isBuiltinDefaultClassConstructor && constructorKind() == ConstructorKind::None));
> }
> 
> void UnlinkedFunctionExecutable::destroy(JSCell* cell)
>@@ -132,7 +134,7 @@ void UnlinkedFunctionExecutable::visitCh
> 
> FunctionExecutable* UnlinkedFunctionExecutable::link(VM& vm, const SourceCode& passedParentSource, std::optional<int> overrideLineNumber, Intrinsic intrinsic)
> {
>-    const SourceCode& parentSource = m_parentSourceOverride.isNull() ? passedParentSource : m_parentSourceOverride;
>+    const SourceCode& parentSource = !m_isBuiltinDefaultClassConstructor ? passedParentSource : BuiltinExecutables::defaultConstructorSourceCode(constructorKind());
>     unsigned firstLine = parentSource.firstLine().oneBasedInt() + m_firstLineOffset;
>     unsigned startOffset = parentSource.startOffset() + m_startOffset;
>     unsigned lineCount = m_lineCount;
>Index: Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.h
>===================================================================
>--- Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.h	(revision 231813)
>+++ Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.h	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2012-2016 Apple Inc. All Rights Reserved.
>+ * Copyright (C) 2012-2018 Apple Inc. All Rights Reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -58,10 +58,10 @@ public:
>     typedef JSCell Base;
>     static const unsigned StructureFlags = Base::StructureFlags | StructureIsImmortal;
> 
>-    static UnlinkedFunctionExecutable* create(VM* vm, const SourceCode& source, FunctionMetadataNode* node, UnlinkedFunctionKind unlinkedFunctionKind, ConstructAbility constructAbility, JSParserScriptMode scriptMode, VariableEnvironment& parentScopeTDZVariables, DerivedContextType derivedContextType, SourceCode&& parentSourceOverride = SourceCode())
>+    static UnlinkedFunctionExecutable* create(VM* vm, const SourceCode& source, FunctionMetadataNode* node, UnlinkedFunctionKind unlinkedFunctionKind, ConstructAbility constructAbility, JSParserScriptMode scriptMode, VariableEnvironment& parentScopeTDZVariables, DerivedContextType derivedContextType, bool isBuiltinDefaultClassConstructor = false)
>     {
>         UnlinkedFunctionExecutable* instance = new (NotNull, allocateCell<UnlinkedFunctionExecutable>(vm->heap))
>-            UnlinkedFunctionExecutable(vm, vm->unlinkedFunctionExecutableStructure.get(), source, WTFMove(parentSourceOverride), node, unlinkedFunctionKind, constructAbility, scriptMode, parentScopeTDZVariables, derivedContextType);
>+            UnlinkedFunctionExecutable(vm, vm->unlinkedFunctionExecutableStructure.get(), source, node, unlinkedFunctionKind, constructAbility, scriptMode, parentScopeTDZVariables, derivedContextType, isBuiltinDefaultClassConstructor);
>         instance->finishCreation(*vm);
>         return instance;
>     }
>@@ -139,7 +139,7 @@ public:
>     void setSourceMappingURLDirective(const String& sourceMappingURL) { m_sourceMappingURLDirective = sourceMappingURL; }
> 
> private:
>-    UnlinkedFunctionExecutable(VM*, Structure*, const SourceCode&, SourceCode&& parentSourceOverride, FunctionMetadataNode*, UnlinkedFunctionKind, ConstructAbility, JSParserScriptMode, VariableEnvironment&,  JSC::DerivedContextType);
>+    UnlinkedFunctionExecutable(VM*, Structure*, const SourceCode&, FunctionMetadataNode*, UnlinkedFunctionKind, ConstructAbility, JSParserScriptMode, VariableEnvironment&,  JSC::DerivedContextType, bool isBuiltinDefaultClassConstructor);
> 
>     unsigned m_firstLineOffset;
>     unsigned m_lineCount;
>@@ -157,6 +157,7 @@ private:
>     unsigned m_isInStrictContext : 1;
>     unsigned m_hasCapturedVariables : 1;
>     unsigned m_isBuiltinFunction : 1;
>+    unsigned m_isBuiltinDefaultClassConstructor : 1;
>     unsigned m_constructAbility: 1;
>     unsigned m_constructorKind : 2;
>     unsigned m_functionMode : 2; // FunctionMode
>@@ -170,7 +171,6 @@ private:
>     Identifier m_name;
>     Identifier m_ecmaName;
>     Identifier m_inferredName;
>-    SourceCode m_parentSourceOverride;
>     SourceCode m_classSource;
> 
>     String m_sourceURLDirective;
>Index: Source/JavaScriptCore/runtime/CodeCache.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/CodeCache.cpp	(revision 231813)
>+++ Source/JavaScriptCore/runtime/CodeCache.cpp	(working copy)
>@@ -150,7 +150,8 @@ UnlinkedFunctionExecutable* CodeCache::g
>     
>     metadata->overrideName(name);
>     metadata->setEndPosition(positionBeforeLastNewline);
>-    // The Function constructor only has access to global variables, so no variables will be under TDZ.
>+    // The Function constructor only has access to global variables, so no variables will be under TDZ unless they're
>+    // in the global lexical environment, which we always TDZ check accesses from.
>     VariableEnvironment emptyTDZVariables;
>     ConstructAbility constructAbility = constructAbilityForParseMode(metadata->parseMode());
>     UnlinkedFunctionExecutable* functionExecutable = UnlinkedFunctionExecutable::create(&vm, source, metadata, UnlinkedNormalFunction, constructAbility, JSParserScriptMode::Classic, emptyTDZVariables, DerivedContextType::None);

Flags:

keith_miller: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 185637: 340444 | 340545