From 6a8e67cadecb4956e9e5e0bb2d793ab538f8aedc Mon Sep 17 00:00:00 2001 From: Francois Doray Date: Wed, 4 Nov 2020 04:51:12 +0000 Subject: [PATCH] Remove obsolete histograms Discarding.(Discards/Reloads)Per10Minutes*. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These histograms are not actively used. Most of the time, no tab is discarded in a 10 minutes interval. To assess the impact of future changes to the discarding logic, it would be more useful to report the number of discards and reloads per day. Bug: 1088709 Change-Id: Ia1b2573c154e0b01c9b6d6039a9b2c041db94b48 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2514467 Reviewed-by: Ilya Sherman Commit-Queue: François Doray Cr-Commit-Position: refs/heads/master@{#823887} --- .../browser/metrics/tab_stats_data_store.cc | 16 ----- chrome/browser/metrics/tab_stats_data_store.h | 18 ------ chrome/browser/metrics/tab_stats_tracker.cc | 60 ------------------- chrome/browser/metrics/tab_stats_tracker.h | 20 +------ .../histograms_xml/others/histograms.xml | 8 +++ 5 files changed, 9 insertions(+), 113 deletions(-) diff --git a/chrome/browser/metrics/tab_stats_data_store.cc b/chrome/browser/metrics/tab_stats_data_store.cc index 49a5db5c1d7ca5..63d420d58e26b6 100644 --- a/chrome/browser/metrics/tab_stats_data_store.cc +++ b/chrome/browser/metrics/tab_stats_data_store.cc @@ -32,8 +32,6 @@ TabStatsDataStore::TabsStats::TabsStats() max_tab_per_window(0U), window_count(0U), window_count_max(0U) { - tab_discard_counts.fill(0U); - tab_reload_counts.fill(0U); } TabStatsDataStore::TabsStats::TabsStats(const TabsStats& other) = default; @@ -168,20 +166,6 @@ void TabStatsDataStore::ResetIntervalData( AddTabToIntervalMap(iter.first, GetTabID(iter.first), true, interval_map); } -void TabStatsDataStore::OnTabDiscardStateChange( - LifecycleUnitDiscardReason discard_reason, - bool is_discarding) { - if (is_discarding) - tab_stats_.tab_discard_counts[static_cast(discard_reason)]++; - else - tab_stats_.tab_reload_counts[static_cast(discard_reason)]++; -} - -void TabStatsDataStore::ClearTabDiscardAndReloadCounts() { - tab_stats_.tab_discard_counts.fill(0U); - tab_stats_.tab_reload_counts.fill(0U); -} - base::Optional TabStatsDataStore::GetTabIDForTesting( content::WebContents* web_contents) { if (!base::Contains(existing_tabs_, web_contents)) diff --git a/chrome/browser/metrics/tab_stats_data_store.h b/chrome/browser/metrics/tab_stats_data_store.h index d04994cca8bda0..aaeb68bf688a4d 100644 --- a/chrome/browser/metrics/tab_stats_data_store.h +++ b/chrome/browser/metrics/tab_stats_data_store.h @@ -53,16 +53,6 @@ class TabStatsDataStore { // The maximum total number of windows opened at the same time. size_t window_count_max; - - // The number of tabs discarded, per discard reason. - std::array(LifecycleUnitDiscardReason::kMaxValue) + 1> - tab_discard_counts; - - // The number of tabs reloaded after a discard, per discard reason. - std::array(LifecycleUnitDiscardReason::kMaxValue) + 1> - tab_reload_counts; }; // Structure describing the state of a tab during an interval of time. @@ -129,14 +119,6 @@ class TabStatsDataStore { // Reset |interval_map| with the list of current tabs. void ResetIntervalData(TabsStateDuringIntervalMap* interval_map); - // Updates counters when the discarded state of a tab changes. - void OnTabDiscardStateChange(LifecycleUnitDiscardReason discard_reason, - bool is_discarding); - - // Clears the discard and reload counters. Called after reporting the counter - // values. - void ClearTabDiscardAndReloadCounts(); - const TabsStats& tab_stats() const { return tab_stats_; } base::Optional GetTabIDForTesting(content::WebContents* web_contents); diff --git a/chrome/browser/metrics/tab_stats_tracker.cc b/chrome/browser/metrics/tab_stats_tracker.cc index bde371db54f8b3..701c32d6686562 100644 --- a/chrome/browser/metrics/tab_stats_tracker.cc +++ b/chrome/browser/metrics/tab_stats_tracker.cc @@ -108,29 +108,6 @@ const char TabStatsTracker::UmaStatsReportingDelegate::kWindowCountHistogramName[] = "Tabs.WindowCount"; -// Tab discard and reload histogram names in the same order as in discard reason -// enum. -const char* kTabDiscardCountHistogramNames[] = { - "Discarding.DiscardsPer10Minutes.Extension", - "Discarding.DiscardsPer10Minutes.Urgent", -}; - -const char* kTabReloadCountHistogramNames[] = { - "Discarding.ReloadsPer10Minutes.Extension", - "Discarding.ReloadsPer10Minutes.Urgent", -}; - -static_assert(base::size(kTabDiscardCountHistogramNames) == - static_cast(LifecycleUnitDiscardReason::kMaxValue) + - 1, - "There must be an entry in kTabDiscardCountHistogramNames for " - "each discard reason."); -static_assert(base::size(kTabReloadCountHistogramNames) == - static_cast(LifecycleUnitDiscardReason::kMaxValue) + - 1, - "There must be an entry in kTabReloadCountHistogramNames for " - "each discard reason."); - const TabStatsDataStore::TabsStats& TabStatsTracker::tab_stats() const { return tab_stats_data_store_->tab_stats(); } @@ -193,21 +170,11 @@ TabStatsTracker::TabStatsTracker(PrefService* pref_service) heartbeat_timer_.Start(FROM_HERE, kTabsHeartbeatReportingInterval, base::BindRepeating(&TabStatsTracker::OnHeartbeatEvent, base::Unretained(this))); - - // Report discarding stats every 10 minutes. - tab_discard_reload_stats_timer_.Start( - FROM_HERE, base::TimeDelta::FromMinutes(10), - base::BindRepeating(&TabStatsTracker::OnTabDiscardCountReportInterval, - base::Unretained(this))); - - g_browser_process->GetTabManager()->AddObserver(this); } TabStatsTracker::~TabStatsTracker() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); BrowserList::GetInstance()->RemoveObserver(this); - - base::PowerMonitor::RemoveObserver(this); } // static @@ -358,19 +325,6 @@ void TabStatsTracker::OnResume() { tab_stats_data_store_->tab_stats().total_tab_count); } -// resource_coordinator::TabLifecycleObserver: -void TabStatsTracker::OnDiscardedStateChange( - content::WebContents* contents, - ::mojom::LifecycleUnitDiscardReason reason, - bool is_discarded) { - // Increment the count in the data store for tabs metrics reporting. - tab_stats_data_store_->OnTabDiscardStateChange(reason, is_discarded); -} - -void TabStatsTracker::OnAutoDiscardableStateChange( - content::WebContents* contents, - bool is_auto_discardable) {} - void TabStatsTracker::OnInterval( base::TimeDelta interval, TabStatsDataStore::TabsStateDuringIntervalMap* interval_map) { @@ -381,20 +335,6 @@ void TabStatsTracker::OnInterval( tab_stats_data_store_->ResetIntervalData(interval_map); } -void TabStatsTracker::OnTabDiscardCountReportInterval() { - for (size_t reason = 0; - reason < static_cast(LifecycleUnitDiscardReason::kMaxValue) + 1; - reason++) { - base::UmaHistogramCounts100( - kTabDiscardCountHistogramNames[reason], - tab_stats_data_store_->tab_stats().tab_discard_counts[reason]); - base::UmaHistogramCounts100( - kTabReloadCountHistogramNames[reason], - tab_stats_data_store_->tab_stats().tab_reload_counts[reason]); - } - tab_stats_data_store_->ClearTabDiscardAndReloadCounts(); -} - void TabStatsTracker::OnInitialOrInsertedTab( content::WebContents* web_contents) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); diff --git a/chrome/browser/metrics/tab_stats_tracker.h b/chrome/browser/metrics/tab_stats_tracker.h index 7afa679b0dbdae..4b6ac3370eecf8 100644 --- a/chrome/browser/metrics/tab_stats_tracker.h +++ b/chrome/browser/metrics/tab_stats_tracker.h @@ -20,8 +20,6 @@ #include "build/build_config.h" #include "chrome/browser/metrics/tab_stats_data_store.h" #include "chrome/browser/metrics/tab_stats_tracker_delegate.h" -#include "chrome/browser/resource_coordinator/lifecycle_unit_state.mojom-forward.h" -#include "chrome/browser/resource_coordinator/tab_lifecycle_observer.h" #include "chrome/browser/ui/browser_list_observer.h" #include "chrome/browser/ui/tabs/tab_strip_model_observer.h" #include "components/metrics/daily_event.h" @@ -43,8 +41,7 @@ FORWARD_DECLARE_TEST(TabStatsTrackerBrowserTest, // std::make_unique(g_browser_process->local_state())); class TabStatsTracker : public TabStripModelObserver, public BrowserListObserver, - public base::PowerObserver, - public resource_coordinator::TabLifecycleObserver { + public base::PowerObserver { public: // Constructor. |pref_service| must outlive this object. explicit TabStatsTracker(PrefService* pref_service); @@ -150,14 +147,6 @@ class TabStatsTracker : public TabStripModelObserver, // base::PowerObserver: void OnResume() override; - // resource_coordinator::TabLifecycleObserver: - void OnDiscardedStateChange(content::WebContents* contents, - ::mojom::LifecycleUnitDiscardReason reason, - bool is_discarded) override; - - void OnAutoDiscardableStateChange(content::WebContents* contents, - bool is_auto_discardable) override; - // Callback when an interval timer triggers. void OnInterval(base::TimeDelta interval, TabStatsDataStore::TabsStateDuringIntervalMap* interval_map); @@ -214,17 +203,10 @@ class TabStatsTracker : public TabStripModelObserver, // The timer used to report the heartbeat metrics at regular interval. base::RepeatingTimer heartbeat_timer_; - // The timer used to report tab discard and reload count histograms at regular - // interval. - base::RepeatingTimer tab_discard_reload_stats_timer_; - // The observers that track how the tabs are used. std::map> web_contents_usage_observers_; - // Called at regular time intervals to report tab discard count histograms. - void OnTabDiscardCountReportInterval(); - SEQUENCE_CHECKER(sequence_checker_); DISALLOW_COPY_AND_ASSIGN(TabStatsTracker); diff --git a/tools/metrics/histograms/histograms_xml/others/histograms.xml b/tools/metrics/histograms/histograms_xml/others/histograms.xml index 8afa1ce17ff037..9af1bdbebbda15 100644 --- a/tools/metrics/histograms/histograms_xml/others/histograms.xml +++ b/tools/metrics/histograms/histograms_xml/others/histograms.xml @@ -3618,6 +3618,10 @@ reviews. Googlers can read more about this at go/gwsq-gerrit. + + Removed 11/2020. Not actively used because reported values are 0 most of the + time. It would be more useful to report number of discards per day. + fdoray@chromium.org @@ -3733,6 +3737,10 @@ reviews. Googlers can read more about this at go/gwsq-gerrit. + + Removed 11/2020. Not actively used because reported values are 0 most of the + time. It would be more useful to report number of reloads per day. + fdoray@chromium.org