From 1d6260396f463f35e7576131f9626112b3a2e0a4 Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Fri, 3 May 2019 10:15:51 +0000 Subject: [PATCH] Bug 1548510 part 2 - Deduplicate GeckoProfilerRuntime::allocProfileString and JitcodeGlobalEntry::createScriptString. r=jonco These functions did the same thing in a slightly different way. Differential Revision: https://phabricator.services.mozilla.com/D29799 --HG-- extra : moz-landing-system : lando --- js/public/ProfilingStack.h | 4 +- js/src/jit/BaselineCompiler.cpp | 4 +- js/src/jit/JitcodeMap.cpp | 96 +------------------- js/src/jit/JitcodeMap.h | 4 - js/src/vm/GeckoProfiler.cpp | 154 +++++++++++++++++++------------- js/src/vm/GeckoProfiler.h | 7 +- js/src/vm/Interpreter.cpp | 2 +- js/src/vm/Probes-inl.h | 4 +- 8 files changed, 106 insertions(+), 169 deletions(-) diff --git a/js/public/ProfilingStack.h b/js/public/ProfilingStack.h index 049822dde199..2935329ad560 100644 --- a/js/public/ProfilingStack.h +++ b/js/public/ProfilingStack.h @@ -553,8 +553,8 @@ class GeckoProfilerThread { * - exit: this function has ceased execution, and no further * entries/exits will be made */ - bool enter(JSContext* cx, JSScript* script, JSFunction* maybeFun); - void exit(JSScript* script, JSFunction* maybeFun); + bool enter(JSContext* cx, JSScript* script); + void exit(JSContext* cx, JSScript* script); inline void updatePC(JSContext* cx, JSScript* script, jsbytecode* pc); }; diff --git a/js/src/jit/BaselineCompiler.cpp b/js/src/jit/BaselineCompiler.cpp index 1c81a1f4079d..938a16fa155a 100644 --- a/js/src/jit/BaselineCompiler.cpp +++ b/js/src/jit/BaselineCompiler.cpp @@ -349,13 +349,13 @@ MethodStatus BaselineCompiler::compile() { baselineScript.get()); // Generate profiling string. - char* str = JitcodeGlobalEntry::createScriptString(cx, script); + UniqueChars str = GeckoProfilerRuntime::allocProfileString(cx, script); if (!str) { return Method_Error; } JitcodeGlobalEntry::BaselineEntry entry; - entry.init(code, code->raw(), code->rawEnd(), script, str); + entry.init(code, code->raw(), code->rawEnd(), script, str.release()); JitcodeGlobalTable* globalTable = cx->runtime()->jitRuntime()->getJitcodeGlobalTable(); diff --git a/js/src/jit/JitcodeMap.cpp b/js/src/jit/JitcodeMap.cpp index 7ba3ce29a474..e9272445fc3e 100644 --- a/js/src/jit/JitcodeMap.cpp +++ b/js/src/jit/JitcodeMap.cpp @@ -276,98 +276,6 @@ int JitcodeGlobalEntry::compare(const JitcodeGlobalEntry& ent1, return flip * -1; } -/* static */ -char* JitcodeGlobalEntry::createScriptString(JSContext* cx, JSScript* script, - size_t* length) { - // If the script has a function, try calculating its name. - bool hasName = false; - size_t nameLength = 0; - UniqueChars nameStr; - JSFunction* func = script->functionDelazifying(); - if (func && func->displayAtom()) { - nameStr = StringToNewUTF8CharsZ(cx, *func->displayAtom()); - if (!nameStr) { - return nullptr; - } - - nameLength = strlen(nameStr.get()); - hasName = true; - } - - // Calculate filename length - const char* filenameStr = script->filename() ? script->filename() : "(null)"; - size_t filenameLength = strlen(filenameStr); - - // Calculate line + column length - bool hasLineAndColumn = false; - size_t lineAndColumnLength = 0; - char lineAndColumnStr[30]; - if (hasName || (script->functionNonDelazifying() || script->isForEval())) { - lineAndColumnLength = SprintfLiteral(lineAndColumnStr, "%u:%u", - script->lineno(), script->column()); - hasLineAndColumn = true; - } - - // Full profile string for scripts with functions is: - // FuncName (FileName:Lineno:Column) - // Full profile string for scripts without functions is: - // FileName:Lineno:Column - // Full profile string for scripts without functions and without lines is: - // FileName - - // Calculate full string length. - size_t fullLength = 0; - if (hasName) { - MOZ_ASSERT(hasLineAndColumn); - fullLength = nameLength + 2 + filenameLength + 1 + lineAndColumnLength + 1; - } else if (hasLineAndColumn) { - fullLength = filenameLength + 1 + lineAndColumnLength; - } else { - fullLength = filenameLength; - } - - // Allocate string. - char* str = cx->pod_malloc(fullLength + 1); - if (!str) { - return nullptr; - } - - size_t cur = 0; - - // Fill string with func name if needed. - if (hasName) { - memcpy(str + cur, nameStr.get(), nameLength); - cur += nameLength; - str[cur++] = ' '; - str[cur++] = '('; - } - - // Fill string with filename chars. - memcpy(str + cur, filenameStr, filenameLength); - cur += filenameLength; - - // Fill line + column chars. - if (hasLineAndColumn) { - str[cur++] = ':'; - memcpy(str + cur, lineAndColumnStr, lineAndColumnLength); - cur += lineAndColumnLength; - } - - // Terminal ')' if necessary. - if (hasName) { - str[cur++] = ')'; - } - - MOZ_ASSERT(cur == fullLength); - str[cur] = 0; - - if (length) { - *length = fullLength; - } - - return str; -} - JitcodeGlobalTable::Enum::Enum(JitcodeGlobalTable& table, JSRuntime* rt) : Range(table), rt_(rt), next_(cur_ ? cur_->tower_->next(0) : nullptr) { for (int level = JitcodeSkiplistTower::MAX_HEIGHT - 1; level >= 0; level--) { @@ -1420,11 +1328,11 @@ bool JitcodeIonTable::makeIonEntry(JSContext* cx, JitCode* code, }); for (uint32_t i = 0; i < numScripts; i++) { - char* str = JitcodeGlobalEntry::createScriptString(cx, scripts[i]); + UniqueChars str = GeckoProfilerRuntime::allocProfileString(cx, scripts[i]); if (!str) { return false; } - if (!profilingStrings.append(str)) { + if (!profilingStrings.append(str.release())) { return false; } } diff --git a/js/src/jit/JitcodeMap.h b/js/src/jit/JitcodeMap.h index 0cbfb18b49fe..3cf040f73718 100644 --- a/js/src/jit/JitcodeMap.h +++ b/js/src/jit/JitcodeMap.h @@ -757,10 +757,6 @@ class JitcodeGlobalEntry { return compare(*this, other); } - // Compute a profiling string for a given script. - static char* createScriptString(JSContext* cx, JSScript* script, - size_t* length = nullptr); - bool hasTrackedOptimizations() const { switch (kind()) { case Ion: diff --git a/js/src/vm/GeckoProfiler.cpp b/js/src/vm/GeckoProfiler.cpp index 6d73ea34c694..ca3ce80850e4 100644 --- a/js/src/vm/GeckoProfiler.cpp +++ b/js/src/vm/GeckoProfiler.cpp @@ -24,6 +24,7 @@ #include "vm/JSScript.h" #include "gc/Marking-inl.h" +#include "vm/JSScript-inl.h" using namespace js; @@ -152,13 +153,17 @@ void GeckoProfilerRuntime::enable(bool enabled) { } /* Lookup the string for the function/script, creating one if necessary */ -const char* GeckoProfilerRuntime::profileString(JSScript* script, - JSFunction* maybeFun) { +const char* GeckoProfilerRuntime::profileString(JSContext* cx, + JSScript* script) { ProfileStringMap::AddPtr s = strings().lookupForAdd(script); if (!s) { - auto str = allocProfileString(script, maybeFun); - if (!str || !strings().add(s, script, std::move(str))) { + UniqueChars str = allocProfileString(cx, script); + if (!str) { + return nullptr; + } + if (!strings().add(s, script, std::move(str))) { + ReportOutOfMemory(cx); return nullptr; } } @@ -187,12 +192,10 @@ void GeckoProfilerRuntime::markEvent(const char* event) { } } -bool GeckoProfilerThread::enter(JSContext* cx, JSScript* script, - JSFunction* maybeFun) { +bool GeckoProfilerThread::enter(JSContext* cx, JSScript* script) { const char* dynamicString = - cx->runtime()->geckoProfiler().profileString(script, maybeFun); + cx->runtime()->geckoProfiler().profileString(cx, script); if (dynamicString == nullptr) { - ReportOutOfMemory(cx); return false; } @@ -214,7 +217,7 @@ bool GeckoProfilerThread::enter(JSContext* cx, JSScript* script, return true; } -void GeckoProfilerThread::exit(JSScript* script, JSFunction* maybeFun) { +void GeckoProfilerThread::exit(JSContext* cx, JSScript* script) { profilingStack_->pop(); #ifdef DEBUG @@ -222,8 +225,7 @@ void GeckoProfilerThread::exit(JSScript* script, JSFunction* maybeFun) { uint32_t sp = profilingStack_->stackPointer; if (sp < profilingStack_->stackCapacity()) { JSRuntime* rt = script->runtimeFromMainThread(); - const char* dynamicString = - rt->geckoProfiler().profileString(script, maybeFun); + const char* dynamicString = rt->geckoProfiler().profileString(cx, script); /* Can't fail lookup because we should already be in the set */ MOZ_ASSERT(dynamicString); @@ -257,65 +259,95 @@ void GeckoProfilerThread::exit(JSScript* script, JSFunction* maybeFun) { * some scripts, resize the hash table of profile strings, and invalidate the * AddPtr held while invoking allocProfileString. */ -UniqueChars GeckoProfilerRuntime::allocProfileString(JSScript* script, - JSFunction* maybeFun) { +/* static */ +UniqueChars GeckoProfilerRuntime::allocProfileString(JSContext* cx, + JSScript* script) { // Note: this profiler string is regexp-matched by // devtools/client/profiler/cleopatra/js/parserWorker.js. - // Get the function name, if any. - JSAtom* atom = maybeFun ? maybeFun->displayAtom() : nullptr; - - // Get the script filename, if any, and its length. - const char* filename = script->filename(); - if (filename == nullptr) { - filename = ""; - } - size_t lenFilename = strlen(filename); - - // Get the line number and its length as a string. - uint32_t lineno = script->lineno(); - size_t lenLineno = 1; - for (uint32_t i = lineno; i /= 10; lenLineno++) - ; - - // Get the column number and its length as a string. - uint32_t column = script->column(); - size_t lenColumn = 1; - for (uint32_t i = column; i /= 10; lenColumn++) - ; - - // Determine the required buffer size. - size_t len = lenFilename + 1 + lenLineno + 1 + - lenColumn; // +1 for each separator colon, ":". - if (atom) { - len += JS::GetDeflatedUTF8StringLength(atom) + - 3; // +3 for the " (" and ")" it adds. - } - - // Allocate the buffer. - UniqueChars cstr(js_pod_malloc(len + 1)); - if (!cstr) { - return nullptr; - } - - // Construct the descriptive string. - DebugOnly ret; - if (atom) { - UniqueChars atomStr = StringToNewUTF8CharsZ(nullptr, *atom); - if (!atomStr) { + // If the script has a function, try calculating its name. + bool hasName = false; + size_t nameLength = 0; + UniqueChars nameStr; + JSFunction* func = script->functionDelazifying(); + if (func && func->displayAtom()) { + nameStr = StringToNewUTF8CharsZ(cx, *func->displayAtom()); + if (!nameStr) { return nullptr; } - ret = snprintf(cstr.get(), len + 1, "%s (%s:%" PRIu32 ":%" PRIu32 ")", - atomStr.get(), filename, lineno, column); - } else { - ret = snprintf(cstr.get(), len + 1, "%s:%" PRIu32 ":%" PRIu32, filename, - lineno, column); + nameLength = strlen(nameStr.get()); + hasName = true; } - MOZ_ASSERT(ret == len, "Computed length should match actual length!"); + // Calculate filename length. + const char* filenameStr = script->filename() ? script->filename() : "(null)"; + size_t filenameLength = strlen(filenameStr); - return cstr; + // Calculate line + column length. + bool hasLineAndColumn = false; + size_t lineAndColumnLength = 0; + char lineAndColumnStr[30]; + if (hasName || script->functionNonDelazifying() || script->isForEval()) { + lineAndColumnLength = SprintfLiteral(lineAndColumnStr, "%u:%u", + script->lineno(), script->column()); + hasLineAndColumn = true; + } + + // Full profile string for scripts with functions is: + // FuncName (FileName:Lineno:Column) + // Full profile string for scripts without functions is: + // FileName:Lineno:Column + // Full profile string for scripts without functions and without lines is: + // FileName + + // Calculate full string length. + size_t fullLength = 0; + if (hasName) { + MOZ_ASSERT(hasLineAndColumn); + fullLength = nameLength + 2 + filenameLength + 1 + lineAndColumnLength + 1; + } else if (hasLineAndColumn) { + fullLength = filenameLength + 1 + lineAndColumnLength; + } else { + fullLength = filenameLength; + } + + // Allocate string. + UniqueChars str(cx->pod_malloc(fullLength + 1)); + if (!str) { + return nullptr; + } + + size_t cur = 0; + + // Fill string with function name if needed. + if (hasName) { + memcpy(str.get() + cur, nameStr.get(), nameLength); + cur += nameLength; + str[cur++] = ' '; + str[cur++] = '('; + } + + // Fill string with filename chars. + memcpy(str.get() + cur, filenameStr, filenameLength); + cur += filenameLength; + + // Fill line + column chars. + if (hasLineAndColumn) { + str[cur++] = ':'; + memcpy(str.get() + cur, lineAndColumnStr, lineAndColumnLength); + cur += lineAndColumnLength; + } + + // Terminal ')' if necessary. + if (hasName) { + str[cur++] = ')'; + } + + MOZ_ASSERT(cur == fullLength); + str[cur] = 0; + + return str; } void GeckoProfilerThread::trace(JSTracer* trc) { diff --git a/js/src/vm/GeckoProfiler.h b/js/src/vm/GeckoProfiler.h index 7ec78246bd57..e44e3f7328fd 100644 --- a/js/src/vm/GeckoProfiler.h +++ b/js/src/vm/GeckoProfiler.h @@ -114,8 +114,6 @@ class GeckoProfilerRuntime { uint32_t enabled_; void (*eventMarker_)(const char*); - UniqueChars allocProfileString(JSScript* script, JSFunction* function); - public: explicit GeckoProfilerRuntime(JSRuntime* rt); @@ -126,7 +124,10 @@ class GeckoProfilerRuntime { bool slowAssertionsEnabled() { return slowAssertions; } void setEventMarker(void (*fn)(const char*)); - const char* profileString(JSScript* script, JSFunction* maybeFun); + + static UniqueChars allocProfileString(JSContext* cx, JSScript* script); + const char* profileString(JSContext* cx, JSScript* script); + void onScriptFinalized(JSScript* script); void markEvent(const char* event); diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index d9c995f4051c..91d9979a4085 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -2000,7 +2000,7 @@ static MOZ_NEVER_INLINE JS_HAZ_JSNATIVE_CALLER bool Interpret(JSContext* cx, // version of the function popped a copy of the frame pushed by the // OSR trampoline.) if (wasProfiler) { - cx->geckoProfiler().exit(script, script->functionNonDelazifying()); + cx->geckoProfiler().exit(cx, script); } if (activation.entryFrame() != REGS.fp()) { diff --git a/js/src/vm/Probes-inl.h b/js/src/vm/Probes-inl.h index 077d8bf59b81..22b4255399bd 100644 --- a/js/src/vm/Probes-inl.h +++ b/js/src/vm/Probes-inl.h @@ -40,7 +40,7 @@ inline bool probes::EnterScript(JSContext* cx, JSScript* script, JSRuntime* rt = cx->runtime(); if (rt->geckoProfiler().enabled()) { - if (!cx->geckoProfiler().enter(cx, script, maybeFun)) { + if (!cx->geckoProfiler().enter(cx, script)) { return false; } MOZ_ASSERT_IF(!fp->script()->isGenerator() && !fp->script()->isAsync(), @@ -64,7 +64,7 @@ inline void probes::ExitScript(JSContext* cx, JSScript* script, #endif if (popProfilerFrame) { - cx->geckoProfiler().exit(script, maybeFun); + cx->geckoProfiler().exit(cx, script); } }