Skip to content

Commit

Permalink
Remove obsolete histograms Discarding.(Discards/Reloads)Per10Minutes*.
Browse files Browse the repository at this point in the history 
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 <isherman@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823887}
  • Loading branch information
@fdoray
fdoray authored and Commit Bot committed Nov 4, 2020
1 parent 8577196 commit 6a8e67c
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 113 deletions.
16 changes: 0 additions & 16 deletions chrome/browser/metrics/tab_stats_data_store.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<size_t>(discard_reason)]++;
else
tab_stats_.tab_reload_counts[static_cast<size_t>(discard_reason)]++;
}

void TabStatsDataStore::ClearTabDiscardAndReloadCounts() {
tab_stats_.tab_discard_counts.fill(0U);
tab_stats_.tab_reload_counts.fill(0U);
}

base::Optional<TabStatsDataStore::TabID> TabStatsDataStore::GetTabIDForTesting(
content::WebContents* web_contents) {
if (!base::Contains(existing_tabs_, web_contents))
Expand Down
18 changes: 0 additions & 18 deletions chrome/browser/metrics/tab_stats_data_store.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<size_t,
static_cast<size_t>(LifecycleUnitDiscardReason::kMaxValue) + 1>
tab_discard_counts;

// The number of tabs reloaded after a discard, per discard reason.
std::array<size_t,
static_cast<size_t>(LifecycleUnitDiscardReason::kMaxValue) + 1>
tab_reload_counts;
};

// Structure describing the state of a tab during an interval of time.
Expand Down Expand Up @@ -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<TabID> GetTabIDForTesting(content::WebContents* web_contents);
Expand Down
60 changes: 0 additions & 60 deletions chrome/browser/metrics/tab_stats_tracker.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<size_t>(LifecycleUnitDiscardReason::kMaxValue) +
1,
"There must be an entry in kTabDiscardCountHistogramNames for "
"each discard reason.");
static_assert(base::size(kTabReloadCountHistogramNames) ==
static_cast<size_t>(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();
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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<size_t>(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_);
Expand Down
20 changes: 1 addition & 19 deletions chrome/browser/metrics/tab_stats_tracker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -43,8 +41,7 @@ FORWARD_DECLARE_TEST(TabStatsTrackerBrowserTest,
// std::make_unique<TabStatsTracker>(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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<content::WebContents*, std::unique_ptr<WebContentsUsageObserver>>
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);
Expand Down
8 changes: 8 additions & 0 deletions tools/metrics/histograms/histograms_xml/others/histograms.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3618,6 +3618,10 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.

<histogram name="Discarding.DiscardsPer10Minutes" units="tabs"
expires_after="M85">
<obsolete>
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.
</obsolete>
<!-- Name completed by histogram_suffixes name="DiscardReason" -->

<owner>fdoray@chromium.org</owner>
Expand Down Expand Up @@ -3733,6 +3737,10 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.

<histogram name="Discarding.ReloadsPer10Minutes" units="tabs"
expires_after="M85">
<obsolete>
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.
</obsolete>
<!-- Name completed by histogram_suffixes name="DiscardReason" -->

<owner>fdoray@chromium.org</owner>
Expand Down

0 comments on commit 6a8e67c

Please sign in to comment.