Bug 841646 - Part 1: Refactor allocProfileString to not require a JSContext and remove JSContext * parameters from functions that no longer require them as a result. r=jandem
authorEmanuel Hoogeveen <emanuel.hoogeveen@gmail.com>
Tue, 28 Jan 2014 08:55:40 -0500
changeset 165597 d52d1a8ea36ccc36ea64988105f151745f01c465
parent 165596 4a801fc0581969d58ccd0d6a0f5cf9528a99d9c8
child 165598 00057fa4863de66587c6716244c6645c2ea56144
push id4623
push userryanvm@gmail.com
push dateTue, 28 Jan 2014 21:48:39 +0000
treeherderfx-team@7e79536aca0a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs841646
milestone29.0a1
Bug 841646 - Part 1: Refactor allocProfileString to not require a JSContext and remove JSContext * parameters from functions that no longer require them as a result. r=jandem
js/src/jit/BaselineBailouts.cpp
js/src/jit/BaselineIC.cpp
js/src/jit/CodeGenerator.cpp
js/src/jit/VMFunctions.cpp
js/src/vm/Probes-inl.h
js/src/vm/SPSProfiler.cpp
js/src/vm/SPSProfiler.h
--- a/js/src/jit/BaselineBailouts.cpp
+++ b/js/src/jit/BaselineBailouts.cpp
@@ -959,17 +959,17 @@ InitFromBailout(JSContext *cx, HandleScr
                     if (caller && bailoutKind == Bailout_ArgumentCheck) {
                         IonSpew(IonSpew_BaselineBailouts, "      Setting PCidx on innermost "
                                 "inlined frame's parent's SPS entry (%s:%d) (pcIdx=%d)!",
                                 caller->filename(), caller->lineno(), caller->pcToOffset(callerPC));
                         cx->runtime()->spsProfiler.updatePC(caller, callerPC);
                     } else if (bailoutKind != Bailout_ArgumentCheck) {
                         IonSpew(IonSpew_BaselineBailouts,
                                 "      Popping SPS entry for innermost inlined frame's SPS entry");
-                        cx->runtime()->spsProfiler.exit(cx, script, fun);
+                        cx->runtime()->spsProfiler.exit(script, fun);
                     }
                 }
             } else {
                 opReturnAddr = nativeCodeForPC;
             }
             builder.setResumeAddr(opReturnAddr);
             IonSpew(IonSpew_BaselineBailouts, "      Set resumeAddr=%p", opReturnAddr);
         }
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -1036,30 +1036,30 @@ DoProfilerFallback(JSContext *cx, Baseli
     mozilla::DebugOnly<ICEntry *> icEntry = stub->icEntry();
 
     FallbackICSpew(cx, stub, "Profiler");
 
     SPSProfiler *profiler = &cx->runtime()->spsProfiler;
 
     // Manually enter SPS this time.
     JS_ASSERT(profiler->enabled());
-    if (!cx->runtime()->spsProfiler.enter(cx, script, func))
+    if (!cx->runtime()->spsProfiler.enter(script, func))
         return false;
     frame->setPushedSPSFrame();
 
     // Unlink any existing PushFunction stub (which may hold stale 'const char *' to
     // the profile string.
     JS_ASSERT_IF(icEntry->firstStub() != stub,
                  icEntry->firstStub()->isProfiler_PushFunction() &&
                  icEntry->firstStub()->next() == stub);
     stub->unlinkStubsWithKind(cx, ICStub::Profiler_PushFunction);
     JS_ASSERT(icEntry->firstStub() == stub);
 
     // Generate the string to use to identify this stack frame.
-    const char *string = profiler->profileString(cx, script, func);
+    const char *string = profiler->profileString(script, func);
     if (string == nullptr)
         return false;
 
     IonSpew(IonSpew_BaselineIC, "  Generating Profiler_PushFunction stub for %s:%d",
             script->filename(), script->lineno());
 
     // Create a new optimized stub.
     ICProfiler_PushFunction::Compiler compiler(cx, string, script);
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -7759,17 +7759,17 @@ CodeGenerator::visitFunctionBoundary(LFu
                 pushArg(ImmGCPtr(lir->script()));
                 if (!callVM(SPSEnterInfo, lir))
                     return false;
                 restoreLive(lir);
                 sps_.pushManual(lir->script(), masm, temp);
                 return true;
             }
 
-            return sps_.push(GetIonContext()->cx, lir->script(), masm, temp);
+            return sps_.push(lir->script(), masm, temp);
 
         case MFunctionBoundary::Inline_Exit:
             // all inline returns were covered with ::Exit, so we just need to
             // maintain the state of inline frames currently active and then
             // reenter the caller
             sps_.leaveInlineFrame();
             sps_.reenter(masm, temp);
             return true;
--- a/js/src/jit/VMFunctions.cpp
+++ b/js/src/jit/VMFunctions.cpp
@@ -521,23 +521,23 @@ JSObject *
 NewStringObject(JSContext *cx, HandleString str)
 {
     return StringObject::create(cx, str);
 }
 
 bool
 SPSEnter(JSContext *cx, HandleScript script)
 {
-    return cx->runtime()->spsProfiler.enter(cx, script, script->functionNonDelazifying());
+    return cx->runtime()->spsProfiler.enter(script, script->functionNonDelazifying());
 }
 
 bool
 SPSExit(JSContext *cx, HandleScript script)
 {
-    cx->runtime()->spsProfiler.exit(cx, script, script->functionNonDelazifying());
+    cx->runtime()->spsProfiler.exit(script, script->functionNonDelazifying());
     return true;
 }
 
 bool
 OperatorIn(JSContext *cx, HandleValue key, HandleObject obj, bool *out)
 {
     RootedId id(cx);
     if (!ValueToId<CanGC>(cx, key, &id))
@@ -727,17 +727,17 @@ DebugEpilogue(JSContext *cx, BaselineFra
         DebugScopes::onPopCall(frame, cx);
     } else if (frame->isStrictEvalFrame()) {
         JS_ASSERT_IF(frame->hasCallObj(), frame->scopeChain()->as<CallObject>().isForEval());
         DebugScopes::onPopStrictEvalScope(frame);
     }
 
     // If the frame has a pushed SPS frame, make sure to pop it.
     if (frame->hasPushedSPSFrame()) {
-        cx->runtime()->spsProfiler.exit(cx, frame->script(), frame->maybeFun());
+        cx->runtime()->spsProfiler.exit(frame->script(), frame->maybeFun());
         // Unset the pushedSPSFrame flag because DebugEpilogue may get called before
         // probes::ExitScript in baseline during exception handling, and we don't
         // want to double-pop SPS frames.
         frame->unsetPushedSPSFrame();
     }
 
     if (!ok) {
         // Pop this frame by updating ionTop, so that the exception handling
--- a/js/src/vm/Probes-inl.h
+++ b/js/src/vm/Probes-inl.h
@@ -49,17 +49,17 @@ probes::EnterScript(JSContext *cx, JSScr
         DTraceEnterJSFun(cx, maybeFun, script);
 #endif
 #ifdef MOZ_TRACE_JSCALLS
     cx->doFunctionCallback(maybeFun, script, 1);
 #endif
 
     JSRuntime *rt = cx->runtime();
     if (rt->spsProfiler.enabled()) {
-        rt->spsProfiler.enter(cx, script, maybeFun);
+        rt->spsProfiler.enter(script, maybeFun);
         JS_ASSERT_IF(!fp->isGeneratorFrame(), !fp->hasPushedSPSFrame());
         fp->setPushedSPSFrame();
     }
 
     return ok;
 }
 
 inline bool
@@ -71,17 +71,17 @@ probes::ExitScript(JSContext *cx, JSScri
     if (JAVASCRIPT_FUNCTION_RETURN_ENABLED())
         DTraceExitJSFun(cx, maybeFun, script);
 #endif
 #ifdef MOZ_TRACE_JSCALLS
     cx->doFunctionCallback(maybeFun, script, 0);
 #endif
 
     if (popSPSFrame)
-        cx->runtime()->spsProfiler.exit(cx, script, maybeFun);
+        cx->runtime()->spsProfiler.exit(script, maybeFun);
 
     return ok;
 }
 
 inline bool
 probes::StartExecution(JSScript *script)
 {
     bool ok = true;
--- a/js/src/vm/SPSProfiler.cpp
+++ b/js/src/vm/SPSProfiler.cpp
@@ -4,16 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "vm/SPSProfiler.h"
 
 #include "mozilla/DebugOnly.h"
 
 #include "jsnum.h"
+#include "jsprf.h"
 #include "jsscript.h"
 
 #include "jit/BaselineJIT.h"
 #include "vm/StringBuffer.h"
 
 using namespace js;
 
 using mozilla::DebugOnly;
@@ -71,23 +72,23 @@ SPSProfiler::enable(bool enabled)
      * their profiler state toggled so they behave properly.
      */
     jit::ToggleBaselineSPS(rt, enabled);
 #endif
 }
 
 /* Lookup the string for the function/script, creating one if necessary */
 const char*
-SPSProfiler::profileString(JSContext *cx, JSScript *script, JSFunction *maybeFun)
+SPSProfiler::profileString(JSScript *script, JSFunction *maybeFun)
 {
     JS_ASSERT(strings.initialized());
     ProfileStringMap::AddPtr s = strings.lookupForAdd(script);
     if (s)
         return s->value();
-    const char *str = allocProfileString(cx, script, maybeFun);
+    const char *str = allocProfileString(script, maybeFun);
     if (str == nullptr)
         return nullptr;
     if (!strings.add(s, script, str)) {
         js_free(const_cast<char *>(str));
         return nullptr;
     }
     return str;
 }
@@ -107,37 +108,37 @@ SPSProfiler::onScriptFinalized(JSScript 
     if (ProfileStringMap::Ptr entry = strings.lookup(script)) {
         const char *tofree = entry->value();
         strings.remove(entry);
         js_free(const_cast<char *>(tofree));
     }
 }
 
 bool
-SPSProfiler::enter(JSContext *cx, JSScript *script, JSFunction *maybeFun)
+SPSProfiler::enter(JSScript *script, JSFunction *maybeFun)
 {
-    const char *str = profileString(cx, script, maybeFun);
+    const char *str = profileString(script, maybeFun);
     if (str == nullptr)
         return false;
 
     JS_ASSERT_IF(*size_ > 0 && *size_ - 1 < max_ && stack_[*size_ - 1].js(),
                  stack_[*size_ - 1].pc() != nullptr);
     push(str, nullptr, script, script->code());
     return true;
 }
 
 void
-SPSProfiler::exit(JSContext *cx, JSScript *script, JSFunction *maybeFun)
+SPSProfiler::exit(JSScript *script, JSFunction *maybeFun)
 {
     pop();
 
 #ifdef DEBUG
     /* Sanity check to make sure push/pop balanced */
     if (*size_ < max_) {
-        const char *str = profileString(cx, script, maybeFun);
+        const char *str = profileString(script, maybeFun);
         /* Can't fail lookup because we should already be in the set */
         JS_ASSERT(str != nullptr);
 
         // Bug 822041
         if (!stack_[*size_].js()) {
             fprintf(stderr, "--- ABOUT TO FAIL ASSERTION ---\n");
             fprintf(stderr, " stack=%p size=%d/%d\n", (void*) stack_, *size_, max_);
             for (int32_t i = *size_; i >= 0; i--) {
@@ -202,54 +203,63 @@ SPSProfiler::pop()
 }
 
 /*
  * Serializes the script/function pair into a "descriptive string" which is
  * allowed to fail. This function cannot trigger a GC because it could finalize
  * some scripts, resize the hash table of profile strings, and invalidate the
  * AddPtr held while invoking allocProfileString.
  */
-const char*
-SPSProfiler::allocProfileString(JSContext *cx, JSScript *script, JSFunction *maybeFun)
+const char *
+SPSProfiler::allocProfileString(JSScript *script, JSFunction *maybeFun)
 {
     // Note: this profiler string is regexp-matched by
     // browser/devtools/profiler/cleopatra/js/parserWorker.js.
-    DebugOnly<uint64_t> gcBefore = cx->runtime()->gcNumber;
-    StringBuffer buf(cx);
-    bool hasAtom = maybeFun != nullptr && maybeFun->displayAtom() != nullptr;
+
+    // Determine if the function (if any) has an explicit or guessed name.
+    bool hasAtom = maybeFun && maybeFun->displayAtom();
+
+    // Get the function name, if any, and its length.
+    const jschar *atom = nullptr;
+    size_t lenAtom = 0;
     if (hasAtom) {
-        if (!buf.append(maybeFun->displayAtom()))
-            return nullptr;
-        if (!buf.append(" ("))
-            return nullptr;
+        atom = maybeFun->displayAtom()->charsZ();
+        lenAtom = maybeFun->displayAtom()->length();
     }
-    if (script->filename()) {
-        if (!buf.appendInflated(script->filename(), strlen(script->filename())))
-            return nullptr;
-    } else if (!buf.append("<unknown>")) {
-        return nullptr;
-    }
-    if (!buf.append(":"))
-        return nullptr;
-    if (!NumberValueToStringBuffer(cx, NumberValue(script->lineno()), buf))
-        return nullptr;
-    if (hasAtom && !buf.append(")"))
-        return nullptr;
+
+    // Get the script filename, if any, and its length.
+    const char *filename = script->filename();
+    if (filename == nullptr)
+        filename = "<unknown>";
+    size_t lenFilename = strlen(filename);
 
-    size_t len = buf.length();
+    // Get the line number and its length as a string.
+    uint64_t lineno = script->lineno();
+    size_t lenLineno = 1;
+    for (uint64_t i = lineno; i /= 10; lenLineno++);
+
+    // Determine the required buffer size.
+    size_t len = lenFilename + lenLineno + 1; // +1 for the ":" separating them.
+    if (hasAtom)
+        len += lenAtom + 3; // +3 for the " (" and ")" it adds.
+
+    // Allocate the buffer.
     char *cstr = js_pod_malloc<char>(len + 1);
     if (cstr == nullptr)
         return nullptr;
 
-    const jschar *ptr = buf.begin();
-    for (size_t i = 0; i < len; i++)
-        cstr[i] = ptr[i];
-    cstr[len] = 0;
+    // Construct the descriptive string.
+    size_t ret;
+    if (hasAtom)
+        ret = JS_snprintf(cstr, len + 1, "%hs (%s:%llu)", atom, filename, lineno);
+    else
+        ret = JS_snprintf(cstr, len + 1, "%s:%llu", filename, lineno);
 
-    JS_ASSERT(gcBefore == cx->runtime()->gcNumber);
+    MOZ_ASSERT(ret == len, "Computed length should match actual length!");
+
     return cstr;
 }
 
 SPSEntryMarker::SPSEntryMarker(JSRuntime *rt
                                MOZ_GUARD_OBJECT_NOTIFIER_PARAM_IN_IMPL)
     : profiler(&rt->spsProfiler)
 {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
--- a/js/src/vm/SPSProfiler.h
+++ b/js/src/vm/SPSProfiler.h
@@ -119,18 +119,17 @@ class SPSProfiler
     JSRuntime            *rt;
     ProfileStringMap     strings;
     ProfileEntry         *stack_;
     uint32_t             *size_;
     uint32_t             max_;
     bool                 slowAssertions;
     uint32_t             enabled_;
 
-    const char *allocProfileString(JSContext *cx, JSScript *script,
-                                   JSFunction *function);
+    const char *allocProfileString(JSScript *script, JSFunction *function);
     void push(const char *string, void *sp, JSScript *script, jsbytecode *pc);
     void pop();
 
   public:
     SPSProfiler(JSRuntime *rt);
     ~SPSProfiler();
 
     uint32_t **addressOfSizePointer() {
@@ -160,34 +159,34 @@ class SPSProfiler
      * Functions which are the actual instrumentation to track run information
      *
      *   - enter: a function has started to execute
      *   - updatePC: updates the pc information about where a function
      *               is currently executing
      *   - exit: this function has ceased execution, and no further
      *           entries/exits will be made
      */
-    bool enter(JSContext *cx, JSScript *script, JSFunction *maybeFun);
-    void exit(JSContext *cx, JSScript *script, JSFunction *maybeFun);
+    bool enter(JSScript *script, JSFunction *maybeFun);
+    void exit(JSScript *script, JSFunction *maybeFun);
     void updatePC(JSScript *script, jsbytecode *pc) {
         if (enabled() && *size_ - 1 < max_) {
             JS_ASSERT(*size_ > 0);
             JS_ASSERT(stack_[*size_ - 1].script() == script);
             stack_[*size_ - 1].setPC(pc);
         }
     }
 
     /* Enter a C++ function. */
     void enterNative(const char *string, void *sp);
     void exitNative() { pop(); }
 
     jsbytecode *ipToPC(JSScript *script, size_t ip) { return nullptr; }
 
     void setProfilingStack(ProfileEntry *stack, uint32_t *size, uint32_t max);
-    const char *profileString(JSContext *cx, JSScript *script, JSFunction *maybeFun);
+    const char *profileString(JSScript *script, JSFunction *maybeFun);
     void onScriptFinalized(JSScript *script);
 
     /* meant to be used for testing, not recommended to call in normal code */
     size_t stringsCount() { return strings.count(); }
     void stringsReset() { strings.clear(); }
 
     uint32_t *addressOfEnabled() {
         return &enabled_;
@@ -315,21 +314,20 @@ class SPSInstrumentation
         JS_ASSERT(frame->left == 0);
         frame->script = script;
     }
 
     /*
      * Flags entry into a JS function for the first time. Before this is called,
      * no instrumentation is emitted, but after this instrumentation is emitted.
      */
-    bool push(JSContext *cx, JSScript *script, Assembler &masm, Register scratch) {
+    bool push(JSScript *script, Assembler &masm, Register scratch) {
         if (!enabled())
             return true;
-        const char *string = profiler_->profileString(cx, script,
-                                                      script->functionNonDelazifying());
+        const char *string = profiler_->profileString(script, script->functionNonDelazifying());
         if (string == nullptr)
             return false;
         masm.spsPushFrame(profiler_, string, script, scratch);
         setPushed(script);
         return true;
     }
 
     /*