Bug 1345145 - Remove some frontend tracelogging that's in functions that are too hot, causing regressions, and whose per-call execution times are too low to be useful. r=h4writer a=gchang
authorShu-yu Guo <shu@rfrn.org>
Thu, 09 Mar 2017 19:24:52 -0800
changeset 395143 87489ca621bdce7da3cc093b9ebff61026914eb6
parent 395142 95a0d4f1b06ec5a96b59c8526fe90e52c40f1abd
child 395144 814932cca0c405b3f95e15fa9b6d4013ccb8cfb1
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersh4writer, gchang
bugs1345145
milestone54.0a2
Bug 1345145 - Remove some frontend tracelogging that's in functions that are too hot, causing regressions, and whose per-call execution times are too low to be useful. r=h4writer a=gchang
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/Parser.cpp
js/src/frontend/TokenStream.cpp
js/src/vm/TraceLogging.cpp
js/src/vm/TraceLoggingTypes.h
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -509,18 +509,16 @@ class BytecodeEmitter::EmitterScope : pu
         return frameSlotEnd() - frameSlotStart();
     }
 
     EmitterScope* enclosingInFrame() const {
         return Nestable<EmitterScope>::enclosing();
     }
 
     NameLocation lookup(BytecodeEmitter* bce, JSAtom* name) {
-        AutoTraceLog traceLog(TraceLoggerForCurrentThread(bce->cx),
-                              TraceLogger_FrontendNameAnalysis);
         if (Maybe<NameLocation> loc = lookupInCache(bce, name))
             return *loc;
         return searchAndCache(bce, name);
     }
 
     Maybe<NameLocation> locationBoundInScope(BytecodeEmitter* bce, JSAtom* name,
                                              EmitterScope* target);
 };
@@ -797,18 +795,16 @@ BytecodeEmitter::EmitterScope::searchAnd
 
     return *loc;
 }
 
 Maybe<NameLocation>
 BytecodeEmitter::EmitterScope::locationBoundInScope(BytecodeEmitter* bce, JSAtom* name,
                                                     EmitterScope* target)
 {
-    AutoTraceLog traceLog(TraceLoggerForCurrentThread(bce->cx), TraceLogger_FrontendNameAnalysis);
-
     // The target scope must be an intra-frame enclosing scope of this
     // one. Count the number of extra hops to reach it.
     uint8_t extraHops = 0;
     for (EmitterScope* es = this; es != target; es = es->enclosingInFrame()) {
         if (es->hasEnvironment())
             extraHops++;
     }
 
@@ -856,18 +852,16 @@ BytecodeEmitter::EmitterScope::deadZoneF
 {
     return deadZoneFrameSlotRange(bce, frameSlotStart(), frameSlotEnd());
 }
 
 bool
 BytecodeEmitter::EmitterScope::enterLexical(BytecodeEmitter* bce, ScopeKind kind,
                                             Handle<LexicalScope::Data*> bindings)
 {
-    AutoTraceLog traceLog(TraceLoggerForCurrentThread(bce->cx), TraceLogger_FrontendNameAnalysis);
-
     MOZ_ASSERT(kind != ScopeKind::NamedLambda && kind != ScopeKind::StrictNamedLambda);
     MOZ_ASSERT(this == bce->innermostEmitterScope);
 
     if (!ensureCache(bce))
         return false;
 
     // Marks all names as closed over if the the context requires it. This
     // cannot be done in the Parser as we may not know if the context requires
@@ -928,18 +922,16 @@ BytecodeEmitter::EmitterScope::enterLexi
         return false;
 
     return checkEnvironmentChainLength(bce);
 }
 
 bool
 BytecodeEmitter::EmitterScope::enterNamedLambda(BytecodeEmitter* bce, FunctionBox* funbox)
 {
-    AutoTraceLog traceLog(TraceLoggerForCurrentThread(bce->cx), TraceLogger_FrontendNameAnalysis);
-
     MOZ_ASSERT(this == bce->innermostEmitterScope);
     MOZ_ASSERT(funbox->namedLambdaBindings());
 
     if (!ensureCache(bce))
         return false;
 
     // See comment in enterLexical about allBindingsClosedOver.
     if (funbox->allBindingsClosedOver())
@@ -997,18 +989,16 @@ BytecodeEmitter::EmitterScope::enterComp
         return false;
 
     return true;
 }
 
 bool
 BytecodeEmitter::EmitterScope::enterParameterExpressionVar(BytecodeEmitter* bce)
 {
-    AutoTraceLog traceLog(TraceLoggerForCurrentThread(bce->cx), TraceLogger_FrontendNameAnalysis);
-
     MOZ_ASSERT(this == bce->innermostEmitterScope);
 
     if (!ensureCache(bce))
         return false;
 
     // Parameter expressions var scopes have no pre-set bindings and are
     // always extensible, as they are needed for eval.
     fallbackFreeNameLocation_ = Some(NameLocation::Dynamic());
@@ -1120,18 +1110,16 @@ BytecodeEmitter::EmitterScope::enterFunc
         return false;
 
     return checkEnvironmentChainLength(bce);
 }
 
 bool
 BytecodeEmitter::EmitterScope::enterFunctionExtraBodyVar(BytecodeEmitter* bce, FunctionBox* funbox)
 {
-    AutoTraceLog traceLog(TraceLoggerForCurrentThread(bce->cx), TraceLogger_FrontendNameAnalysis);
-
     MOZ_ASSERT(funbox->hasParameterExprs);
     MOZ_ASSERT(funbox->extraVarScopeBindings() ||
                funbox->needsExtraBodyVarEnvironmentRegardlessOfBindings());
     MOZ_ASSERT(this == bce->innermostEmitterScope);
 
     // The extra var scope is never popped once it's entered. It replaces the
     // function scope as the var emitter scope.
     bce->setVarEmitterScope(this);
@@ -1211,18 +1199,16 @@ class DynamicBindingIter : public Bindin
             MOZ_CRASH("Bad BindingKind");
         }
     }
 };
 
 bool
 BytecodeEmitter::EmitterScope::enterGlobal(BytecodeEmitter* bce, GlobalSharedContext* globalsc)
 {
-    AutoTraceLog traceLog(TraceLoggerForCurrentThread(bce->cx), TraceLogger_FrontendNameAnalysis);
-
     MOZ_ASSERT(this == bce->innermostEmitterScope);
 
     bce->setVarEmitterScope(this);
 
     if (!ensureCache(bce))
         return false;
 
     if (bce->emitterMode == BytecodeEmitter::SelfHosting) {
@@ -1273,18 +1259,16 @@ BytecodeEmitter::EmitterScope::enterGlob
         return GlobalScope::create(cx, globalsc->scopeKind(), globalsc->bindings);
     };
     return internBodyScope(bce, createScope);
 }
 
 bool
 BytecodeEmitter::EmitterScope::enterEval(BytecodeEmitter* bce, EvalSharedContext* evalsc)
 {
-    AutoTraceLog traceLog(TraceLoggerForCurrentThread(bce->cx), TraceLogger_FrontendNameAnalysis);
-
     MOZ_ASSERT(this == bce->innermostEmitterScope);
 
     bce->setVarEmitterScope(this);
 
     if (!ensureCache(bce))
         return false;
 
     // For simplicity, treat all free name lookups in eval scripts as dynamic.
@@ -1330,18 +1314,16 @@ BytecodeEmitter::EmitterScope::enterEval
     }
 
     return true;
 }
 
 bool
 BytecodeEmitter::EmitterScope::enterModule(BytecodeEmitter* bce, ModuleSharedContext* modulesc)
 {
-    AutoTraceLog traceLog(TraceLoggerForCurrentThread(bce->cx), TraceLogger_FrontendNameAnalysis);
-
     MOZ_ASSERT(this == bce->innermostEmitterScope);
 
     bce->setVarEmitterScope(this);
 
     if (!ensureCache(bce))
         return false;
 
     // Resolve body-level bindings, if there are any.
@@ -1389,18 +1371,16 @@ BytecodeEmitter::EmitterScope::enterModu
         return false;
 
     return checkEnvironmentChainLength(bce);
 }
 
 bool
 BytecodeEmitter::EmitterScope::enterWith(BytecodeEmitter* bce)
 {
-    AutoTraceLog traceLog(TraceLoggerForCurrentThread(bce->cx), TraceLogger_FrontendNameAnalysis);
-
     MOZ_ASSERT(this == bce->innermostEmitterScope);
 
     if (!ensureCache(bce))
         return false;
 
     // 'with' make all accesses dynamic and unanalyzable.
     fallbackFreeNameLocation_ = Some(NameLocation::Dynamic());
 
@@ -1417,18 +1397,16 @@ BytecodeEmitter::EmitterScope::enterWith
         return false;
 
     return checkEnvironmentChainLength(bce);
 }
 
 bool
 BytecodeEmitter::EmitterScope::leave(BytecodeEmitter* bce, bool nonLocal)
 {
-    AutoTraceLog traceLog(TraceLoggerForCurrentThread(bce->cx), TraceLogger_FrontendNameAnalysis);
-
     // If we aren't leaving the scope due to a non-local jump (e.g., break),
     // we must be the innermost scope.
     MOZ_ASSERT_IF(!nonLocal, this == bce->innermostEmitterScope);
 
     ScopeKind kind = scope(bce)->kind();
     switch (kind) {
       case ScopeKind::Lexical:
       case ScopeKind::SimpleCatch:
@@ -1476,18 +1454,16 @@ BytecodeEmitter::EmitterScope::leave(Byt
     }
 
     return true;
 }
 
 Maybe<MaybeCheckTDZ>
 BytecodeEmitter::TDZCheckCache::needsTDZCheck(BytecodeEmitter* bce, JSAtom* name)
 {
-    AutoTraceLog traceLog(TraceLoggerForCurrentThread(bce->cx), TraceLogger_FrontendTDZAnalysis);
-
     if (!ensureCache(bce))
         return Nothing();
 
     CheckTDZMap::AddPtr p = cache_->lookupForAdd(name);
     if (p)
         return Some(p->value().wrapped);
 
     MaybeCheckTDZ rv = CheckTDZ;
@@ -1507,18 +1483,16 @@ BytecodeEmitter::TDZCheckCache::needsTDZ
 
     return Some(rv);
 }
 
 bool
 BytecodeEmitter::TDZCheckCache::noteTDZCheck(BytecodeEmitter* bce, JSAtom* name,
                                              MaybeCheckTDZ check)
 {
-    AutoTraceLog traceLog(TraceLoggerForCurrentThread(bce->cx), TraceLogger_FrontendTDZAnalysis);
-
     if (!ensureCache(bce))
         return false;
 
     CheckTDZMap::AddPtr p = cache_->lookupForAdd(name);
     if (p) {
         MOZ_ASSERT(!check, "TDZ only needs to be checked once per binding per basic block.");
         p->value() = check;
     } else {
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -1052,18 +1052,16 @@ Parser<ParseHandler>::reportRedeclaratio
 // forbid duplicates.)
 template <typename ParseHandler>
 bool
 Parser<ParseHandler>::notePositionalFormalParameter(Node fn, HandlePropertyName name,
                                                     uint32_t beginPos,
                                                     bool disallowDuplicateParams,
                                                     bool* duplicatedParam)
 {
-    AutoTraceLog traceLog(TraceLoggerForCurrentThread(context), TraceLogger_FrontendNameAnalysis);
-
     if (AddDeclaredNamePtr p = pc->functionScope().lookupDeclaredNameForAdd(name)) {
         if (disallowDuplicateParams) {
             error(JSMSG_BAD_DUP_ARGS);
             return false;
         }
 
         // Strict-mode disallows duplicate args. We may not know whether we are
         // in strict mode or not (since the function body hasn't been parsed).
@@ -1285,18 +1283,16 @@ Parser<ParseHandler>::tryDeclareVar(Hand
     return true;
 }
 
 template <typename ParseHandler>
 bool
 Parser<ParseHandler>::tryDeclareVarForAnnexBLexicalFunction(HandlePropertyName name,
                                                             uint32_t beginPos, bool* tryAnnexB)
 {
-    AutoTraceLog traceLog(TraceLoggerForCurrentThread(context), TraceLogger_FrontendNameAnalysis);
-
     Maybe<DeclarationKind> redeclaredKind;
     uint32_t unused;
     if (!tryDeclareVar(name, DeclarationKind::VarForAnnexBLexicalFunction, beginPos,
                        &redeclaredKind, &unused))
     {
         return false;
     }
 
@@ -1355,18 +1351,16 @@ Parser<ParseHandler>::checkLexicalDeclar
     return true;
 }
 
 template <typename ParseHandler>
 bool
 Parser<ParseHandler>::noteDeclaredName(HandlePropertyName name, DeclarationKind kind,
                                        TokenPos pos)
 {
-    AutoTraceLog traceLog(TraceLoggerForCurrentThread(context), TraceLogger_FrontendNameAnalysis);
-
     // The asm.js validator does all its own symbol-table management so, as an
     // optimization, avoid doing any work here.
     if (pc->useAsmOrInsideUseAsm())
         return true;
 
     switch (kind) {
       case DeclarationKind::Var:
       case DeclarationKind::BodyLevelFunction:
@@ -1528,18 +1522,16 @@ Parser<ParseHandler>::noteDeclaredName(H
 
     return true;
 }
 
 template <typename ParseHandler>
 bool
 Parser<ParseHandler>::noteUsedName(HandlePropertyName name)
 {
-    AutoTraceLog traceLog(TraceLoggerForCurrentThread(context), TraceLogger_FrontendNameAnalysis);
-
     // If the we are delazifying, the LazyScript already has all the
     // closed-over info for bindings and there's no need to track used names.
     if (handler.canSkipLazyClosedOverBindings())
         return true;
 
     // The asm.js validator does all its own symbol-table management so, as an
     // optimization, avoid doing any work here.
     if (pc->useAsmOrInsideUseAsm())
@@ -1555,29 +1547,25 @@ Parser<ParseHandler>::noteUsedName(Handl
 
     return usedNames.noteUse(context, name, pc->scriptId(), scope->id());
 }
 
 template <typename ParseHandler>
 bool
 Parser<ParseHandler>::hasUsedName(HandlePropertyName name)
 {
-    AutoTraceLog traceLog(TraceLoggerForCurrentThread(context), TraceLogger_FrontendNameAnalysis);
-
     if (UsedNamePtr p = usedNames.lookup(name))
         return p->value().isUsedInScript(pc->scriptId());
     return false;
 }
 
 template <typename ParseHandler>
 bool
 Parser<ParseHandler>::propagateFreeNamesAndMarkClosedOverBindings(ParseContext::Scope& scope)
 {
-    AutoTraceLog traceLog(TraceLoggerForCurrentThread(context), TraceLogger_FrontendNameAnalysis);
-
     if (handler.canSkipLazyClosedOverBindings()) {
         // Scopes are nullptr-delimited in the LazyScript closed over bindings
         // array.
         while (JSAtom* name = handler.nextLazyClosedOverBinding())
             scope.lookupDeclaredName(name)->value()->setClosedOver();
         return true;
     }
 
--- a/js/src/frontend/TokenStream.cpp
+++ b/js/src/frontend/TokenStream.cpp
@@ -1297,20 +1297,16 @@ static const uint8_t firstCharKinds[] = 
 #undef _______
 
 static_assert(LastCharKind < (1 << (sizeof(firstCharKinds[0]) * 8)),
               "Elements of firstCharKinds[] are too small");
 
 bool
 TokenStream::getTokenInternal(TokenKind* ttp, Modifier modifier)
 {
-    // The assumption is that the cost of other tokenizer code is dwarfed by
-    // this one.
-    AutoTraceLog tokenizerLog(TraceLoggerForCurrentThread(cx), TraceLogger_Tokenizing);
-
     int c;
     uint32_t qc;
     Token* tp;
     FirstCharKind c1kind;
     const char16_t* numStart;
     bool hasExp;
     DecimalPoint decimalPoint;
     const char16_t* identStart;
--- a/js/src/vm/TraceLogging.cpp
+++ b/js/src/vm/TraceLogging.cpp
@@ -826,21 +826,18 @@ TraceLoggerThreadState::init()
         enabledTextIds[TraceLogger_RegisterAllocation] = true;
         enabledTextIds[TraceLogger_GenerateCode] = true;
         enabledTextIds[TraceLogger_Scripts] = true;
         enabledTextIds[TraceLogger_IonBuilderRestartLoop] = true;
     }
 
     if (ContainsFlag(env, "Frontend")) {
         enabledTextIds[TraceLogger_Frontend] = true;
-        enabledTextIds[TraceLogger_FrontendNameAnalysis] = true;
-        enabledTextIds[TraceLogger_FrontendTDZAnalysis] = true;
         enabledTextIds[TraceLogger_ParsingFull] = true;
         enabledTextIds[TraceLogger_ParsingSyntax] = true;
-        enabledTextIds[TraceLogger_Tokenizing] = true;
         enabledTextIds[TraceLogger_BytecodeEmission] = true;
         enabledTextIds[TraceLogger_BytecodeFoldConstants] = true;
         enabledTextIds[TraceLogger_BytecodeNameFunctions] = true;
     }
 
     enabledTextIds[TraceLogger_Interpreter] = enabledTextIds[TraceLogger_Engine];
     enabledTextIds[TraceLogger_Baseline] = enabledTextIds[TraceLogger_Engine];
     enabledTextIds[TraceLogger_IonMonkey] = enabledTextIds[TraceLogger_Engine];
--- a/js/src/vm/TraceLoggingTypes.h
+++ b/js/src/vm/TraceLoggingTypes.h
@@ -25,21 +25,18 @@
     _(IonCompilation)                                 \
     _(IonCompilationPaused)                           \
     _(IonLinking)                                     \
     _(IonMonkey)                                      \
     _(IrregexpCompile)                                \
     _(IrregexpExecute)                                \
     _(MinorGC)                                        \
     _(Frontend)                                       \
-    _(FrontendNameAnalysis)                           \
-    _(FrontendTDZAnalysis)                            \
     _(ParsingFull)                                    \
     _(ParsingSyntax)                                  \
-    _(Tokenizing)                                     \
     _(BytecodeEmission)                               \
     _(BytecodeFoldConstants)                          \
     _(BytecodeNameFunctions)                          \
     _(DecodeScript)                                   \
     _(DecodeFunction)                                 \
     _(EncodeScript)                                   \
     _(EncodeFunction)                                 \
     _(Scripts)                                        \