From 38d0922abdd309c8c5c412087703e9c959fe5f1a Mon Sep 17 00:00:00 2001 From: Aaron Klotz Date: Tue, 30 Dec 2014 14:52:39 -0700 Subject: [PATCH] Bug 1100360: Convert ChromeHang annotations to use UniquePtr; r=vladan --HG-- extra : rebase_source : 5c2ae8645b927c4d73a649beb573cfe2e242c146 --- toolkit/components/telemetry/Telemetry.cpp | 68 +++++++++++++--------- toolkit/components/telemetry/Telemetry.h | 3 +- xpcom/threads/HangMonitor.cpp | 13 ++--- 3 files changed, 47 insertions(+), 37 deletions(-) diff --git a/toolkit/components/telemetry/Telemetry.cpp b/toolkit/components/telemetry/Telemetry.cpp index fd47c602400c..de5d88e437f7 100644 --- a/toolkit/components/telemetry/Telemetry.cpp +++ b/toolkit/components/telemetry/Telemetry.cpp @@ -234,26 +234,37 @@ public: */ struct AnnotationInfo { AnnotationInfo(uint32_t aHangIndex, - HangAnnotations* aAnnotations) + UniquePtr aAnnotations) : mHangIndex(aHangIndex) - , mAnnotations(aAnnotations) + , mAnnotations(Move(aAnnotations)) {} - AnnotationInfo(const AnnotationInfo& aOther) + AnnotationInfo(AnnotationInfo&& aOther) : mHangIndex(aOther.mHangIndex) - , mAnnotations(aOther.mAnnotations) + , mAnnotations(Move(aOther.mAnnotations)) {} ~AnnotationInfo() {} + AnnotationInfo& operator=(AnnotationInfo&& aOther) + { + mHangIndex = aOther.mHangIndex; + mAnnotations = Move(aOther.mAnnotations); + return *this; + } uint32_t mHangIndex; - mutable nsAutoPtr mAnnotations; + UniquePtr mAnnotations; + + private: + // Force move constructor + AnnotationInfo(const AnnotationInfo& aOther) MOZ_DELETE; + void operator=(const AnnotationInfo& aOther) MOZ_DELETE; }; size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const; void AddHang(const Telemetry::ProcessedStack& aStack, uint32_t aDuration, int32_t aSystemUptime, int32_t aFirefoxUptime, - HangAnnotations* aAnnotations); + UniquePtr aAnnotations); uint32_t GetDuration(unsigned aIndex) const; int32_t GetSystemUptime(unsigned aIndex) const; int32_t GetFirefoxUptime(unsigned aIndex) const; - const std::vector& GetAnnotationInfo() const; + const nsTArray& GetAnnotationInfo() const; const CombinedStacks& GetStacks() const; private: /** @@ -269,7 +280,7 @@ private: int32_t mFirefoxUptime; }; std::vector mHangInfo; - std::vector mAnnotationInfo; + nsTArray mAnnotationInfo; CombinedStacks mStacks; }; @@ -278,13 +289,13 @@ HangReports::AddHang(const Telemetry::ProcessedStack& aStack, uint32_t aDuration, int32_t aSystemUptime, int32_t aFirefoxUptime, - HangAnnotations* aAnnotations) { + UniquePtr aAnnotations) { HangInfo info = { aDuration, aSystemUptime, aFirefoxUptime }; mHangInfo.push_back(info); if (aAnnotations) { AnnotationInfo ainfo(static_cast(mHangInfo.size() - 1), - aAnnotations); - mAnnotationInfo.push_back(ainfo); + Move(aAnnotations)); + mAnnotationInfo.AppendElement(Move(ainfo)); } mStacks.AddStack(aStack); } @@ -296,10 +307,9 @@ HangReports::SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const { // This is a crude approximation. See comment on // CombinedStacks::SizeOfExcludingThis. n += mHangInfo.capacity() * sizeof(HangInfo); - n += mAnnotationInfo.capacity() * sizeof(AnnotationInfo); - for (std::vector::const_iterator i = mAnnotationInfo.begin(), - e = mAnnotationInfo.end(); i != e; ++i) { - n += i->mAnnotations->SizeOfIncludingThis(aMallocSizeOf); + n += mAnnotationInfo.Capacity() * sizeof(AnnotationInfo); + for (int32_t i = 0, l = mAnnotationInfo.Length(); i < l; ++i) { + n += mAnnotationInfo[i].mAnnotations->SizeOfIncludingThis(aMallocSizeOf); } return n; } @@ -324,7 +334,7 @@ HangReports::GetFirefoxUptime(unsigned aIndex) const { return mHangInfo[aIndex].mFirefoxUptime; } -const std::vector& +const nsTArray& HangReports::GetAnnotationInfo() const { return mAnnotationInfo; } @@ -816,7 +826,7 @@ public: Telemetry::ProcessedStack &aStack, int32_t aSystemUptime, int32_t aFirefoxUptime, - HangAnnotations* aAnnotations); + UniquePtr aAnnotations); #endif static void RecordThreadHangStats(Telemetry::ThreadHangStats& aStats); static nsresult GetHistogramEnumId(const char *name, Telemetry::ID *id); @@ -2405,18 +2415,16 @@ TelemetryImpl::GetChromeHangs(JSContext *cx, JS::MutableHandle ret) if (!JS_SetElement(cx, firefoxUptimeArray, i, mHangReports.GetFirefoxUptime(i))) { return NS_ERROR_FAILURE; } - const std::vector& annotationInfo = + const nsTArray& annotationInfo = mHangReports.GetAnnotationInfo(); - uint32_t annotationsArrayIndex = 0; - for (std::vector::const_iterator - ai = annotationInfo.begin(), e = annotationInfo.end(); ai != e; - ++ai, ++annotationsArrayIndex) { + for (uint32_t iterIndex = 0, arrayLen = annotationInfo.Length(); + iterIndex < arrayLen; ++iterIndex) { JS::Rooted keyValueArray(cx, JS_NewArrayObject(cx, 0)); if (!keyValueArray) { return NS_ERROR_FAILURE; } JS::RootedValue indexValue(cx); - indexValue.setNumber(ai->mHangIndex); + indexValue.setNumber(annotationInfo[iterIndex].mHangIndex); if (!JS_SetElement(cx, keyValueArray, 0, indexValue)) { return NS_ERROR_FAILURE; } @@ -2427,7 +2435,8 @@ TelemetryImpl::GetChromeHangs(JSContext *cx, JS::MutableHandle ret) return NS_ERROR_FAILURE; } nsAutoPtr annotationsEnum; - if (!ai->mAnnotations->GetEnumerator(annotationsEnum.StartAssignment())) { + if (!annotationInfo[iterIndex].mAnnotations->GetEnumerator( + annotationsEnum.StartAssignment())) { return NS_ERROR_FAILURE; } nsAutoString key; @@ -2443,7 +2452,7 @@ TelemetryImpl::GetChromeHangs(JSContext *cx, JS::MutableHandle ret) if (!JS_SetElement(cx, keyValueArray, 1, jsAnnotation)) { return NS_ERROR_FAILURE; } - if (!JS_SetElement(cx, annotationsArray, annotationsArrayIndex, + if (!JS_SetElement(cx, annotationsArray, iterIndex, keyValueArray)) { return NS_ERROR_FAILURE; } @@ -3200,7 +3209,7 @@ TelemetryImpl::RecordChromeHang(uint32_t aDuration, Telemetry::ProcessedStack &aStack, int32_t aSystemUptime, int32_t aFirefoxUptime, - HangAnnotations* aAnnotations) + UniquePtr aAnnotations) { if (!sTelemetry || !sTelemetry->mCanRecord) return; @@ -3209,7 +3218,7 @@ TelemetryImpl::RecordChromeHang(uint32_t aDuration, sTelemetry->mHangReports.AddHang(aStack, aDuration, aSystemUptime, aFirefoxUptime, - aAnnotations); + Move(aAnnotations)); } #endif @@ -3473,10 +3482,11 @@ void RecordChromeHang(uint32_t duration, ProcessedStack &aStack, int32_t aSystemUptime, int32_t aFirefoxUptime, - HangAnnotations* aAnnotations) + UniquePtr aAnnotations) { TelemetryImpl::RecordChromeHang(duration, aStack, - aSystemUptime, aFirefoxUptime, aAnnotations); + aSystemUptime, aFirefoxUptime, + Move(aAnnotations)); } #endif diff --git a/toolkit/components/telemetry/Telemetry.h b/toolkit/components/telemetry/Telemetry.h index ae30fd7d5537..f572ef0e61ed 100644 --- a/toolkit/components/telemetry/Telemetry.h +++ b/toolkit/components/telemetry/Telemetry.h @@ -238,7 +238,8 @@ void RecordChromeHang(uint32_t aDuration, ProcessedStack &aStack, int32_t aSystemUptime, int32_t aFirefoxUptime, - mozilla::HangMonitor::HangAnnotations* aAnnotations = nullptr); + mozilla::UniquePtr + aAnnotations); #endif class ThreadHangStats; diff --git a/xpcom/threads/HangMonitor.cpp b/xpcom/threads/HangMonitor.cpp index 58805c567559..a2e3d7b4f81d 100644 --- a/xpcom/threads/HangMonitor.cpp +++ b/xpcom/threads/HangMonitor.cpp @@ -8,14 +8,14 @@ #include +#include "mozilla/Atomics.h" #include "mozilla/BackgroundHangMonitor.h" #include "mozilla/Monitor.h" #include "mozilla/Preferences.h" -#include "mozilla/Telemetry.h" #include "mozilla/ProcessedStack.h" -#include "mozilla/Atomics.h" +#include "mozilla/Telemetry.h" #include "mozilla/StaticPtr.h" -#include "nsAutoPtr.h" +#include "mozilla/UniquePtr.h" #include "nsReadableUtils.h" #include "nsStackWalk.h" #include "nsThreadUtils.h" @@ -363,7 +363,7 @@ ThreadMain(void*) Telemetry::ProcessedStack stack; int32_t systemUptime = -1; int32_t firefoxUptime = -1; - nsAutoPtr annotations = new ChromeHangAnnotations(); + auto annotations = MakeUnique(); #endif while (true) { @@ -410,10 +410,9 @@ ThreadMain(void*) if (waitCount >= 2) { uint32_t hangDuration = PR_IntervalToSeconds(now - lastTimestamp); Telemetry::RecordChromeHang(hangDuration, stack, systemUptime, - firefoxUptime, annotations->IsEmpty() ? - nullptr : annotations.forget()); + firefoxUptime, Move(annotations)); stack.Clear(); - annotations = new ChromeHangAnnotations(); + annotations = MakeUnique(); } #endif lastTimestamp = timestamp;