From 3476c1d60ec29c5497123194acd7a9310b1023d2 Mon Sep 17 00:00:00 2001 From: David Teller Date: Mon, 28 Jan 2019 23:41:20 +0100 Subject: Reduce number of allocations in AutoStopwatch This patch fixes two related issues. 1. The AutoStopwatch uses a stack-allocated `mozilla::Vector` to communicate with its callback during each compartment switch. This vector was designed to allow its contents to be stack-allocated but they turned out to be accidentally heap-allocated. 2. During each tick, the stopwatch fills a vector `recentGroups_`. This vector always started with minimal capacity and had to grow repeatedly as groups were added, causing repeated reallocations. This patch preallocates `recentGroups_` to have the same capacity as the previous tick. We expect that this should eventually reach a stable size that closely matches the actual needs of the process. --- toolkit/components/perfmonitoring/nsPerformanceStats.cpp | 8 ++++++-- toolkit/components/perfmonitoring/nsPerformanceStats.h | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) (limited to 'toolkit/components') diff --git a/toolkit/components/perfmonitoring/nsPerformanceStats.cpp b/toolkit/components/perfmonitoring/nsPerformanceStats.cpp index 6c470356ac..03e63a4618 100644 --- a/toolkit/components/perfmonitoring/nsPerformanceStats.cpp +++ b/toolkit/components/perfmonitoring/nsPerformanceStats.cpp @@ -1310,8 +1310,12 @@ nsPerformanceStatsService::GetResources(uint64_t* userTime, void nsPerformanceStatsService::NotifyJankObservers(const mozilla::Vector& aPreviousJankLevels) { - GroupVector alerts; - mPendingAlerts.swap(alerts); + + // The move operation is generally constant time, unless `mPendingAlerts.length()` is very small, in which case it's + // fast anyway. + GroupVector alerts(Move(mPendingAlerts)); + mPendingAlerts = GroupVector(); // Reconstruct after `Move`. + if (!mPendingAlertsCollector) { // We are shutting down. return; diff --git a/toolkit/components/perfmonitoring/nsPerformanceStats.h b/toolkit/components/perfmonitoring/nsPerformanceStats.h index 6902c840d6..661a78a1ab 100644 --- a/toolkit/components/perfmonitoring/nsPerformanceStats.h +++ b/toolkit/components/perfmonitoring/nsPerformanceStats.h @@ -19,7 +19,7 @@ class nsPerformanceGroup; class nsPerformanceGroupDetails; -typedef mozilla::Vector> GroupVector; +typedef mozilla::Vector, 8> GroupVector; /** * A data structure for registering observers interested in -- cgit v1.2.3 From abcaa560fcaf2f814fc40eef46557033c910eb96 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Tue, 29 Jan 2019 00:40:24 +0100 Subject: Revert "Reduce number of allocations in AutoStopwatch" This reverts commit 3476c1d60ec29c5497123194acd7a9310b1023d2. --- toolkit/components/perfmonitoring/nsPerformanceStats.cpp | 8 ++------ toolkit/components/perfmonitoring/nsPerformanceStats.h | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) (limited to 'toolkit/components') diff --git a/toolkit/components/perfmonitoring/nsPerformanceStats.cpp b/toolkit/components/perfmonitoring/nsPerformanceStats.cpp index 03e63a4618..6c470356ac 100644 --- a/toolkit/components/perfmonitoring/nsPerformanceStats.cpp +++ b/toolkit/components/perfmonitoring/nsPerformanceStats.cpp @@ -1310,12 +1310,8 @@ nsPerformanceStatsService::GetResources(uint64_t* userTime, void nsPerformanceStatsService::NotifyJankObservers(const mozilla::Vector& aPreviousJankLevels) { - - // The move operation is generally constant time, unless `mPendingAlerts.length()` is very small, in which case it's - // fast anyway. - GroupVector alerts(Move(mPendingAlerts)); - mPendingAlerts = GroupVector(); // Reconstruct after `Move`. - + GroupVector alerts; + mPendingAlerts.swap(alerts); if (!mPendingAlertsCollector) { // We are shutting down. return; diff --git a/toolkit/components/perfmonitoring/nsPerformanceStats.h b/toolkit/components/perfmonitoring/nsPerformanceStats.h index 661a78a1ab..6902c840d6 100644 --- a/toolkit/components/perfmonitoring/nsPerformanceStats.h +++ b/toolkit/components/perfmonitoring/nsPerformanceStats.h @@ -19,7 +19,7 @@ class nsPerformanceGroup; class nsPerformanceGroupDetails; -typedef mozilla::Vector, 8> GroupVector; +typedef mozilla::Vector> GroupVector; /** * A data structure for registering observers interested in -- cgit v1.2.3 From b55d41c240df13812760a2a77f086a477f450fd0 Mon Sep 17 00:00:00 2001 From: David Teller Date: Tue, 29 Jan 2019 03:11:39 +0100 Subject: Reduce number of allocations in AutoStopwatch This patch fixes two related issues. 1. The AutoStopwatch uses a stack-allocated `mozilla::Vector` to communicate with its callback during each compartment switch. This vector was designed to allow its contents to be stack-allocated but they turned out to be accidentally heap-allocated. 2. During each tick, the stopwatch fills a vector `recentGroups_`. This vector always started with minimal capacity and had to grow repeatedly as groups were added, causing repeated reallocations. This patch preallocates `recentGroups_` to have the same capacity as the previous tick. We expect that this should eventually reach a stable size that closely matches the actual needs of the process. --- toolkit/components/perfmonitoring/nsPerformanceStats.cpp | 11 +++++++++-- toolkit/components/perfmonitoring/nsPerformanceStats.h | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) (limited to 'toolkit/components') diff --git a/toolkit/components/perfmonitoring/nsPerformanceStats.cpp b/toolkit/components/perfmonitoring/nsPerformanceStats.cpp index 6c470356ac..59d84ced15 100644 --- a/toolkit/components/perfmonitoring/nsPerformanceStats.cpp +++ b/toolkit/components/perfmonitoring/nsPerformanceStats.cpp @@ -1082,6 +1082,9 @@ nsPerformanceStatsService::GetPerformanceGroups(JSContext* cx, return false; } + // Returning a vector that is too large would cause allocations all over the + // place in the JS engine. We want to be sure that all data is stored inline. + MOZ_ASSERT(out.length() <= out.sMaxInlineStorage); return true; } @@ -1310,8 +1313,12 @@ nsPerformanceStatsService::GetResources(uint64_t* userTime, void nsPerformanceStatsService::NotifyJankObservers(const mozilla::Vector& aPreviousJankLevels) { - GroupVector alerts; - mPendingAlerts.swap(alerts); + + // The move operation is generally constant time, unless + // `mPendingAlerts.length()` is very small, in which case it's fast anyway. + GroupVector alerts(Move(mPendingAlerts)); + mPendingAlerts = GroupVector(); // Reconstruct after `Move`. + if (!mPendingAlertsCollector) { // We are shutting down. return; diff --git a/toolkit/components/perfmonitoring/nsPerformanceStats.h b/toolkit/components/perfmonitoring/nsPerformanceStats.h index 6902c840d6..661a78a1ab 100644 --- a/toolkit/components/perfmonitoring/nsPerformanceStats.h +++ b/toolkit/components/perfmonitoring/nsPerformanceStats.h @@ -19,7 +19,7 @@ class nsPerformanceGroup; class nsPerformanceGroupDetails; -typedef mozilla::Vector> GroupVector; +typedef mozilla::Vector, 8> GroupVector; /** * A data structure for registering observers interested in -- cgit v1.2.3