author | Julien Wajsberg <felash@gmail.com> |
Mon, 08 Jan 2018 18:25:36 +0100 | |
changeset 400737 | fcf73947d244a6e44652435ad1eb5993ceab4de6 |
parent 400736 | 60a59819cf322121940c1de7f511483444e3199e |
child 400738 | 167ef8dc0c60a49c9c7de5cdba48c276dec5d355 |
push id | 33315 |
push user | rgurzau@mozilla.com |
push date | Thu, 25 Jan 2018 17:00:51 +0000 |
treeherder | mozilla-central@53c5a199232a [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | mstange |
bugs | 1428076 |
milestone | 60.0a1 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/tools/profiler/core/ProfileBuffer.h +++ b/tools/profiler/core/ProfileBuffer.h @@ -44,21 +44,21 @@ public: void CollectCodeLocation( const char* aLabel, const char* aStr, int aLineNumber, const mozilla::Maybe<js::ProfileEntry::Category>& aCategory); // Maximum size of a frameKey string that we'll handle. static const size_t kMaxFrameKeyLength = 512; - void StreamSamplesToJSON(SpliceableJSONWriter& aWriter, int aThreadId, + bool StreamSamplesToJSON(SpliceableJSONWriter& aWriter, int aThreadId, double aSinceTime, double* aOutFirstSampleTime, JSContext* cx, UniqueStacks& aUniqueStacks) const; - void StreamMarkersToJSON(SpliceableJSONWriter& aWriter, int aThreadId, + bool StreamMarkersToJSON(SpliceableJSONWriter& aWriter, int aThreadId, const mozilla::TimeStamp& aProcessStartTime, double aSinceTime, UniqueStacks& aUniqueStacks) const; void StreamPausedRangesToJSON(SpliceableJSONWriter& aWriter, double aSinceTime) const; // Find (via |aLS|) the most recent sample for the thread denoted by // |aThreadId| and clone it, patching in |aProcessStartTime| as appropriate.
--- a/tools/profiler/core/ProfileBufferEntry.cpp +++ b/tools/profiler/core/ProfileBufferEntry.cpp @@ -695,17 +695,18 @@ private: // DynamicStringFragment("://perf-") // DynamicStringFragment("html.io/") // DynamicStringFragment("ac0da204") // DynamicStringFragment("aaa44d75") // DynamicStringFragment("a800.bun") // DynamicStringFragment("dle.js:2") // DynamicStringFragment("5)") // -void +// This method returns true if it wrote anything to the writer. +bool ProfileBuffer::StreamSamplesToJSON(SpliceableJSONWriter& aWriter, int aThreadId, double aSinceTime, double* aOutFirstSampleTime, JSContext* aContext, UniqueStacks& aUniqueStacks) const { UniquePtr<char[]> strbuf = MakeUnique<char[]>(kMaxFrameKeyLength); @@ -716,16 +717,17 @@ ProfileBuffer::StreamSamplesToJSON(Splic { \ fprintf(stderr, "ProfileBuffer parse error: %s", msg); \ MOZ_ASSERT(false, msg); \ continue; \ } EntryGetter e(*this); bool seenFirstSample = false; + bool haveSamples = false; for (;;) { // This block skips entries until we find the start of the next sample. // This is useful in three situations. // // - The circular buffer overwrites old entries, so when we start parsing // we might be in the middle of a sample, and we must skip forward to the // start of the next sample. @@ -899,46 +901,53 @@ ProfileBuffer::StreamSamplesToJSON(Splic } if (e.Has() && e.Get().IsUnsharedMemory()) { sample.mUSS = Some(e.Get().u.mDouble); e.Next(); } WriteSample(aWriter, sample); + haveSamples = true; } + return haveSamples; #undef ERROR_AND_CONTINUE } -void +// This method returns true if it wrote anything to the writer. +bool ProfileBuffer::StreamMarkersToJSON(SpliceableJSONWriter& aWriter, int aThreadId, const TimeStamp& aProcessStartTime, double aSinceTime, UniqueStacks& aUniqueStacks) const { EntryGetter e(*this); + bool haveMarkers = false; // Stream all markers whose threadId matches aThreadId. We skip other entries, // because we process them in StreamSamplesToJSON(). // // NOTE: The ThreadId of a marker is determined by its GetThreadId() method, // rather than ThreadId buffer entries, as markers can be added outside of // samples. while (e.Has()) { if (e.Get().IsMarker()) { const ProfilerMarker* marker = e.Get().u.mMarker; if (marker->GetTime() >= aSinceTime && marker->GetThreadId() == aThreadId) { marker->StreamJSON(aWriter, aProcessStartTime, aUniqueStacks); + haveMarkers = true; } } e.Next(); } + + return haveMarkers; } static void AddPausedRange(SpliceableJSONWriter& aWriter, const char* aReason, const Maybe<double>& aStartTime, const Maybe<double>& aEndTime) { aWriter.Start(); if (aStartTime) {
--- a/tools/profiler/core/ThreadInfo.cpp +++ b/tools/profiler/core/ThreadInfo.cpp @@ -90,19 +90,19 @@ ThreadInfo::StreamJSON(const ProfileBuff aProcessStartTime, mRegisterTime, mUnregisterTime, aSinceTime, &firstSampleTime, mContext, mSavedStreamedSamples.get(), mFirstSavedStreamedSampleTime, mSavedStreamedMarkers.get(), *mUniqueStacks); - mSavedStreamedSamples.reset(); + mSavedStreamedSamples = nullptr; mFirstSavedStreamedSampleTime = 0.0; - mSavedStreamedMarkers.reset(); + mSavedStreamedMarkers = nullptr; aWriter.StartObjectProperty("stackTable"); { { JSONSchemaWriter schema(aWriter); schema.WriteField("prefix"); schema.WriteField("frame"); } @@ -253,34 +253,74 @@ ThreadInfo::FlushSamplesAndMarkers(const // // Note that the UniqueStacks instance is persisted so that the frame-index // mapping is stable across JS shutdown. mUniqueStacks.emplace(mContext); { SpliceableChunkedJSONWriter b; b.StartBareList(); + bool haveSamples = false; { - aBuffer.StreamSamplesToJSON(b, ThreadId(), /* aSinceTime = */ 0, - &mFirstSavedStreamedSampleTime, - mContext, *mUniqueStacks); + if (mSavedStreamedSamples) { + b.Splice(mSavedStreamedSamples.get()); + haveSamples = true; + } + + // We deliberately use a new variable instead of writing something like + // `haveSamples || aBuffer.StreamSamplesToJSON(...)` because we don't want + // to short-circuit the call. + bool streamedNewSamples = + aBuffer.StreamSamplesToJSON(b, ThreadId(), /* aSinceTime = */ 0, + &mFirstSavedStreamedSampleTime, + mContext, *mUniqueStacks); + haveSamples = haveSamples || streamedNewSamples; } b.EndBareList(); - mSavedStreamedSamples = b.WriteFunc()->CopyData(); + + // https://bugzilla.mozilla.org/show_bug.cgi?id=1428076 + // If we don't have any data, keep mSavedStreamSamples set to null. That + // way we won't try to splice it into the JSON later on, which would + // result in an invalid JSON due to stray commas. + if (haveSamples) { + mSavedStreamedSamples = b.WriteFunc()->CopyData(); + } else { + mSavedStreamedSamples = nullptr; + } } { SpliceableChunkedJSONWriter b; b.StartBareList(); + bool haveMarkers = false; { - aBuffer.StreamMarkersToJSON(b, ThreadId(), aProcessStartTime, - /* aSinceTime = */ 0, *mUniqueStacks); + if (mSavedStreamedMarkers) { + b.Splice(mSavedStreamedMarkers.get()); + haveMarkers = true; + } + + // We deliberately use a new variable instead of writing something like + // `haveMarkers || aBuffer.StreamMarkersToJSON(...)` because we don't want + // to short-circuit the call. + bool streamedNewMarkers = + aBuffer.StreamMarkersToJSON(b, ThreadId(), aProcessStartTime, + /* aSinceTime = */ 0, *mUniqueStacks); + haveMarkers = haveMarkers || streamedNewMarkers; } b.EndBareList(); - mSavedStreamedMarkers = b.WriteFunc()->CopyData(); + + // https://bugzilla.mozilla.org/show_bug.cgi?id=1428076 + // If we don't have any data, keep mSavedStreamMarkers set to null. That + // way we won't try to splice it into the JSON later on, which would + // result in an invalid JSON due to stray commas. + if (haveMarkers) { + mSavedStreamedMarkers = b.WriteFunc()->CopyData(); + } else { + mSavedStreamedMarkers = nullptr; + } } // Reset the buffer. Attempting to symbolicate JS samples after mContext has // gone away will crash. aBuffer.Reset(); } size_t