Commit cccdd9fb authored by Nathan Froyd's avatar Nathan Froyd
Browse files

Bug 1177488 - use |const char*| for representing async call reasons; r=bz,fitzgen

Using a simple |const char*| is more memory-efficient than allocating a
JS string. We still have to allocate the JS string for passing things
into JS, but ideally we will be able to move the point of allocation
much closer to where it's actually needed, rather than indiscriminantly
doing it all the time.
parent 16d14681
Loading
Loading
Loading
Loading
+2 −1
Original line number Diff line number Diff line
@@ -14108,13 +14108,14 @@ nsDocShell::GetOpener()
  return opener;
}

// The caller owns |aAsyncCause| here.
void
nsDocShell::NotifyJSRunToCompletionStart(const char* aReason,
                                         const char16_t* aFunctionName,
                                         const char16_t* aFilename,
                                         const uint32_t aLineNumber,
                                         JS::Handle<JS::Value> aAsyncStack,
                                         JS::Handle<JS::Value> aAsyncCause)
                                         const char* aAsyncCause)
{
  // If first start, mark interval start.
  if (mJSRunToCompletionDepth == 0) {
+1 −1
Original line number Diff line number Diff line
@@ -1059,7 +1059,7 @@ interface nsIDocShell : nsIDocShellTreeItem
                                                                  in wstring fileName,
                                                                  in unsigned long lineNumber,
                                                                  in jsval asyncStack,
                                                                  in jsval asyncCause);
                                                                  in string asyncCause);
  [noscript,notxpcom,nostdcall] void notifyJSRunToCompletionStop();

  /**
+13 −5
Original line number Diff line number Diff line
@@ -17,23 +17,25 @@ namespace mozilla {
class JavascriptTimelineMarker : public TimelineMarker
{
public:
  // The caller owns |aAsyncCause| here, so we must copy it into a separate
  // string for use later on.
  JavascriptTimelineMarker(const char* aReason,
                           const char16_t* aFunctionName,
                           const char16_t* aFileName,
                           uint32_t aLineNumber,
                           MarkerTracingType aTracingType,
                           JS::Handle<JS::Value> aAsyncStack,
                           JS::Handle<JS::Value> aAsyncCause)
                           const char* aAsyncCause)
    : TimelineMarker("Javascript", aTracingType, MarkerStackRequest::NO_STACK)
    , mCause(NS_ConvertUTF8toUTF16(aReason))
    , mFunctionName(aFunctionName)
    , mFileName(aFileName)
    , mLineNumber(aLineNumber)
    , mAsyncCause(aAsyncCause)
  {
    JSContext* ctx = nsContentUtils::GetCurrentJSContext();
    if (ctx) {
      mAsyncStack.init(ctx, aAsyncStack);
      mAsyncCause.init(ctx, aAsyncCause);
    }
  }

@@ -50,10 +52,16 @@ public:
      stackFrame.mFunctionDisplayName.Construct(mFunctionName);

      if (mAsyncStack.isObject() && !mAsyncStack.isNullOrUndefined() &&
          mAsyncCause.isString()) {
          !mAsyncCause.IsEmpty()) {
        JS::Rooted<JSObject*> asyncStack(aCx, mAsyncStack.toObjectOrNull());
        JS::Rooted<JSString*> asyncCause(aCx, mAsyncCause.toString());
        JS::Rooted<JSObject*> parentFrame(aCx);
        JS::Rooted<JSString*> asyncCause(aCx, JS_NewUCStringCopyN(aCx, mAsyncCause.BeginReading(),
                                                                  mAsyncCause.Length()));
        if (!asyncCause) {
          JS_ClearPendingException(aCx);
          return;
        }

        if (!JS::CopyAsyncStack(aCx, asyncStack, asyncCause, &parentFrame, 0)) {
          JS_ClearPendingException(aCx);
        } else {
@@ -78,7 +86,7 @@ private:
  nsString mFileName;
  uint32_t mLineNumber;
  JS::PersistentRooted<JS::Value> mAsyncStack;
  JS::PersistentRooted<JS::Value> mAsyncCause;
  NS_ConvertUTF8toUTF16 mAsyncCause;
};

} // namespace mozilla
+2 −4
Original line number Diff line number Diff line
@@ -683,7 +683,7 @@ AutoEntryScript::DocshellEntryMonitor::DocshellEntryMonitor(JSContext* aCx,
void
AutoEntryScript::DocshellEntryMonitor::Entry(JSContext* aCx, JSFunction* aFunction,
                                             JSScript* aScript, JS::Handle<JS::Value> aAsyncStack,
                                             JS::Handle<JSString*> aAsyncCause)
                                             const char* aAsyncCause)
{
  JS::Rooted<JSFunction*> rootedFunction(aCx);
  if (aFunction) {
@@ -728,13 +728,11 @@ AutoEntryScript::DocshellEntryMonitor::Entry(JSContext* aCx, JSFunction* aFuncti
    const char16_t* functionNameChars = functionName.isTwoByte() ?
      functionName.twoByteChars() : nullptr;

    JS::Rooted<JS::Value> asyncCauseValue(aCx, aAsyncCause ? StringValue(aAsyncCause) :
                                          JS::NullValue());
    docShellForJSRunToCompletion->NotifyJSRunToCompletionStart(mReason,
                                                               functionNameChars,
                                                               filename.BeginReading(),
                                                               lineNumber, aAsyncStack,
                                                               asyncCauseValue);
                                                               aAsyncCause);
  }
}

+8 −3
Original line number Diff line number Diff line
@@ -356,16 +356,21 @@ private:
  public:
    DocshellEntryMonitor(JSContext* aCx, const char* aReason);

    // Please note that |aAsyncCause| here is owned by the caller, and its
    // lifetime must outlive the lifetime of the DocshellEntryMonitor object.
    // In practice, |aAsyncCause| is identical to |aReason| passed into
    // the AutoEntryScript constructor, so the lifetime requirements are
    // trivially satisfied by |aReason| being a statically allocated string.
    void Entry(JSContext* aCx, JSFunction* aFunction,
               JS::Handle<JS::Value> aAsyncStack,
               JS::Handle<JSString*> aAsyncCause) override
               const char* aAsyncCause) override
    {
      Entry(aCx, aFunction, nullptr, aAsyncStack, aAsyncCause);
    }

    void Entry(JSContext* aCx, JSScript* aScript,
               JS::Handle<JS::Value> aAsyncStack,
               JS::Handle<JSString*> aAsyncCause) override
               const char* aAsyncCause) override
    {
      Entry(aCx, nullptr, aScript, aAsyncStack, aAsyncCause);
    }
@@ -375,7 +380,7 @@ private:
  private:
    void Entry(JSContext* aCx, JSFunction* aFunction, JSScript* aScript,
               JS::Handle<JS::Value> aAsyncStack,
               JS::Handle<JSString*> aAsyncCause);
               const char* aAsyncCause);

    const char* mReason;
  };
Loading