Attachment 361499 Details for Bug 194432 – Patch

[patch] Patch

bug-194432-20190208025204.patch (text/plain), 8.38 KB, created by Yusuke Suzuki on 2019-02-08 02:52:04 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-02-08 02:52:04 PST

Size: 8.38 KB

patch

obsolete

>Subversion Revision: 241169
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 1a08fea7c578a822a7e8194a2b6220555548a836..584e147b6abec124cb23b165f14cffbe8e61d85c 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,22 @@
>+2019-02-08  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] SourceProviderCacheItem should be small
>+        https://bugs.webkit.org/show_bug.cgi?id=194432
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Some JetStream2 tests stress the JS parser. At that time, so many SourceProviderCacheItems are created.
>+        While they are removed when full-GC happens, it significantly increases the peak memory usage.
>+        This patch reduces the size of SourceProviderCacheItem from 56 to 32.
>+
>+        * parser/Parser.cpp:
>+        (JSC::Parser<LexerType>::parseFunctionInfo):
>+        * parser/ParserModes.h:
>+        * parser/ParserTokens.h:
>+        * parser/SourceProviderCacheItem.h:
>+        (JSC::SourceProviderCacheItem::endFunctionToken const):
>+        (JSC::SourceProviderCacheItem::SourceProviderCacheItem):
>+
> 2019-02-07  Mark Lam  <mark.lam@apple.com>
> 
>         Fix more doesGC() for CheckTraps, GetMapBucket, and Switch nodes.
>diff --git a/Source/JavaScriptCore/parser/Parser.cpp b/Source/JavaScriptCore/parser/Parser.cpp
>index 8c1466c5c6193f15007f8d03eca54dbad6dc44ce..d840a7fda4c095fff583178861d054f1730f249c 100644
>--- a/Source/JavaScriptCore/parser/Parser.cpp
>+++ b/Source/JavaScriptCore/parser/Parser.cpp
>@@ -2598,7 +2598,6 @@ template <class TreeBuilder> bool Parser<LexerType>::parseFunctionInfo(TreeBuild
>     if (TreeBuilder::CanUseFunctionCache && m_functionCache && sourceLength > minimumSourceLengthToCache) {
>         SourceProviderCacheItemCreationParameters parameters;
>         parameters.endFunctionOffset = functionInfo.endOffset;
>-        parameters.functionNameStart = functionNameStart;
>         parameters.lastTokenLine = location.line;
>         parameters.lastTokenStartOffset = location.startOffset;
>         parameters.lastTokenEndOffset = location.endOffset;
>diff --git a/Source/JavaScriptCore/parser/ParserModes.h b/Source/JavaScriptCore/parser/ParserModes.h
>index 9e637c4aa09537a456eb7ad30d80547b3a2aa002..e2a08b16501a4a81f521576e19615e7ace9a938e 100644
>--- a/Source/JavaScriptCore/parser/ParserModes.h
>+++ b/Source/JavaScriptCore/parser/ParserModes.h
>@@ -328,4 +328,5 @@ const InnerArrowFunctionCodeFeatures SuperPropertyInnerArrowFunctionFeature = 1
> const InnerArrowFunctionCodeFeatures NewTargetInnerArrowFunctionFeature =     1 << 5;
>     
> const InnerArrowFunctionCodeFeatures AllInnerArrowFunctionCodeFeatures = EvalInnerArrowFunctionFeature | ArgumentsInnerArrowFunctionFeature | ThisInnerArrowFunctionFeature | SuperCallInnerArrowFunctionFeature | SuperPropertyInnerArrowFunctionFeature | NewTargetInnerArrowFunctionFeature;
>+static_assert(AllInnerArrowFunctionCodeFeatures <= 0b111111, "InnerArrowFunctionCodeFeatures must be 6bits");
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/parser/ParserTokens.h b/Source/JavaScriptCore/parser/ParserTokens.h
>index 59cb0fb4c868de0ccfc2da83f59492523560fe02..cfee33798c24d8783bd99c48e1c87208873b5c9c 100644
>--- a/Source/JavaScriptCore/parser/ParserTokens.h
>+++ b/Source/JavaScriptCore/parser/ParserTokens.h
>@@ -41,6 +41,8 @@ enum {
>     // P = binary operator precedence
>     // K = keyword flag
>     // U = unary operator flag
>+    //
>+    // We must keep the upper 8bit (1byte) region empty. JSTokenType must be 24bits.
>     UnaryOpTokenFlag = 128,
>     KeywordTokenFlag = 256,
>     BinaryOpTokenPrecedenceShift = 9,
>@@ -189,6 +191,7 @@ enum JSTokenType {
>     INVALID_TEMPLATE_LITERAL_ERRORTOK = 15 | ErrorTokenFlag,
>     UNEXPECTED_ESCAPE_ERRORTOK = 16 | ErrorTokenFlag,
> };
>+static_assert(static_cast<unsigned>(POW) <= 0x00ffffffU, "JSTokenType must be 24bits.");
> 
> struct JSTextPosition {
>     JSTextPosition() = default;
>diff --git a/Source/JavaScriptCore/parser/SourceProviderCacheItem.h b/Source/JavaScriptCore/parser/SourceProviderCacheItem.h
>index d0644d2df17195ad7e24b5cb5c3725ac821660f4..7775b69e9bdcd00955cb909b8a139734b8ff5640 100644
>--- a/Source/JavaScriptCore/parser/SourceProviderCacheItem.h
>+++ b/Source/JavaScriptCore/parser/SourceProviderCacheItem.h
>@@ -34,14 +34,12 @@
> namespace JSC {
> 
> struct SourceProviderCacheItemCreationParameters {
>-    unsigned functionNameStart;
>     unsigned lastTokenLine;
>     unsigned lastTokenStartOffset;
>     unsigned lastTokenEndOffset;
>     unsigned lastTokenLineStartOffset;
>     unsigned endFunctionOffset;
>     unsigned parameterCount;
>-    unsigned functionLength;
>     bool needsFullActivation;
>     bool usesEval;
>     bool strictMode;
>@@ -68,7 +66,7 @@ class SourceProviderCacheItem {
>     JSToken endFunctionToken() const 
>     {
>         JSToken token;
>-        token.m_type = isBodyArrowExpression ? tokenType : CLOSEBRACE;
>+        token.m_type = isBodyArrowExpression ? static_cast<JSTokenType>(tokenType) : CLOSEBRACE;
>         token.m_data.offset = lastTokenStartOffset;
>         token.m_location.startOffset = lastTokenStartOffset;
>         token.m_location.endOffset = lastTokenEndOffset;
>@@ -79,24 +77,22 @@ class SourceProviderCacheItem {
>         return token;
>     }
> 
>-    unsigned functionNameStart : 31;
>     bool needsFullActivation : 1;
>     unsigned endFunctionOffset : 31;
>     bool usesEval : 1;
>     unsigned lastTokenLine : 31;
>     bool strictMode : 1;
>     unsigned lastTokenStartOffset : 31;
>-    unsigned lastTokenEndOffset: 31;
>-    unsigned constructorKind : 2; // ConstructorKind
>-    unsigned parameterCount : 31;
>     unsigned expectedSuperBinding : 1; // SuperBinding
>+    unsigned lastTokenEndOffset: 31;
>     bool needsSuperBinding: 1;
>-    unsigned functionLength;
>-    unsigned lastTokenLineStartOffset;
>+    unsigned parameterCount : 31;
>+    unsigned lastTokenLineStartOffset : 31;
>+    bool isBodyArrowExpression : 1;
>     unsigned usedVariablesCount;
>-    InnerArrowFunctionCodeFeatures innerArrowFunctionFeatures;
>-    bool isBodyArrowExpression;
>-    JSTokenType tokenType;
>+    unsigned tokenType : 24; // JSTokenType
>+    unsigned innerArrowFunctionFeatures : 6; // InnerArrowFunctionCodeFeatures
>+    unsigned constructorKind : 2; // ConstructorKind
> 
>     UniquedStringImpl** usedVariables() const { return const_cast<UniquedStringImpl**>(m_variables); }
> 
>@@ -121,25 +117,27 @@ inline std::unique_ptr<SourceProviderCacheItem> SourceProviderCacheItem::create(
> }
> 
> inline SourceProviderCacheItem::SourceProviderCacheItem(const SourceProviderCacheItemCreationParameters& parameters)
>-    : functionNameStart(parameters.functionNameStart)
>-    , needsFullActivation(parameters.needsFullActivation)
>+    : needsFullActivation(parameters.needsFullActivation)
>     , endFunctionOffset(parameters.endFunctionOffset)
>     , usesEval(parameters.usesEval)
>     , lastTokenLine(parameters.lastTokenLine)
>     , strictMode(parameters.strictMode)
>     , lastTokenStartOffset(parameters.lastTokenStartOffset)
>-    , lastTokenEndOffset(parameters.lastTokenEndOffset)
>-    , constructorKind(static_cast<unsigned>(parameters.constructorKind))
>-    , parameterCount(parameters.parameterCount)
>     , expectedSuperBinding(static_cast<unsigned>(parameters.expectedSuperBinding))
>+    , lastTokenEndOffset(parameters.lastTokenEndOffset)
>     , needsSuperBinding(parameters.needsSuperBinding)
>-    , functionLength(parameters.functionLength)
>+    , parameterCount(parameters.parameterCount)
>     , lastTokenLineStartOffset(parameters.lastTokenLineStartOffset)
>-    , usedVariablesCount(parameters.usedVariables.size())
>-    , innerArrowFunctionFeatures(parameters.innerArrowFunctionFeatures)
>     , isBodyArrowExpression(parameters.isBodyArrowExpression)
>-    , tokenType(parameters.tokenType)
>+    , usedVariablesCount(parameters.usedVariables.size())
>+    , tokenType(static_cast<unsigned>(parameters.tokenType))
>+    , innerArrowFunctionFeatures(static_cast<unsigned>(parameters.innerArrowFunctionFeatures))
>+    , constructorKind(static_cast<unsigned>(parameters.constructorKind))
> {
>+    ASSERT(tokenType == static_cast<unsigned>(parameters.tokenType));
>+    ASSERT(innerArrowFunctionFeatures == static_cast<unsigned>(parameters.innerArrowFunctionFeatures));
>+    ASSERT(constructorKind == static_cast<unsigned>(parameters.constructorKind));
>+    ASSERT(expectedSuperBinding == static_cast<unsigned>(parameters.expectedSuperBinding));
>     for (unsigned i = 0; i < usedVariablesCount; ++i) {
>         m_variables[i] = parameters.usedVariables[i];
>         m_variables[i]->ref();

Flags:

saam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 194432: 361499