Skip to content

GitLab

T
tangled
Commit 7144e845 authored by Brian Sheedy's avatar Brian Sheedy Committed by Commit Bot
Browse files
Options

Revert "Add metrics for thumbnail memory usage" 

This reverts commit f5d2f7eb.

Reason for revert: Suspected cause of crbug.com/1137028

Original change's description:
> Add metrics for thumbnail memory usage
>
> Adds two metrics logged every 5 minutes:
> * Per-thumbnail compressed data memory usage
> * Total compressed data memory usage
>
> The former has a sample for each thumbnail every 5 minutes, while the
> latter has exactly one every 5 minutes.
>
> Bug: 11261624
> Change-Id: I1595c6a6b771f73e3be9a5497d1cf6f6ec3d1af7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2462685
> Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
> Reviewed-by: Dana Fried <dfried@chromium.org>
> Commit-Queue: Collin Baker <collinbaker@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#815714}

TBR=rkaplow@chromium.org,dfried@chromium.org,collinbaker@chromium.org

Change-Id: I3c0532e5653c3240715a17c00b9aec69bc323e38
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 11261624, 1137028
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2463488Reviewed-by: default avatarBrian Sheedy <bsheedy@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815795}
parent 89a0f3b5
Hide whitespace changes
Inline Side-by-side
Showing with 0 additions and 330 deletions
chrome/browser/ui/BUILD.gn
View file @ 7144e845
......@@ -1256,8 +1256,6 @@ static_library("ui") {
"thumbnails/thumbnail_scheduler.h",
"thumbnails/thumbnail_scheduler_impl.cc",
"thumbnails/thumbnail_scheduler_impl.h",
"thumbnails/thumbnail_stats_tracker.cc",
"thumbnails/thumbnail_stats_tracker.h",
"thumbnails/thumbnail_tab_helper.cc",
"thumbnails/thumbnail_tab_helper.h",
"toolbar/app_menu_icon_controller.cc",
......
chrome/browser/ui/thumbnails/thumbnail_image.cc
View file @ 7144e845
......@@ -11,7 +11,6 @@
#include "base/task/task_traits.h"
#include "base/task/thread_pool.h"
#include "base/time/time.h"
#include "chrome/browser/ui/thumbnails/thumbnail_stats_tracker.h"
#include "ui/gfx/codec/jpeg_codec.h"
#include "ui/gfx/skia_util.h"
......@@ -36,12 +35,10 @@ ThumbnailImage::ThumbnailImage(Delegate* delegate) : delegate_(delegate) {
DCHECK(delegate_);
DCHECK(!delegate_->thumbnail_);
delegate_->thumbnail_ = this;
ThumbnailStatsTracker::GetInstance().AddThumbnail(this);
}
ThumbnailImage::~ThumbnailImage() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
ThumbnailStatsTracker::GetInstance().RemoveThumbnail(this);
if (delegate_)
delegate_->thumbnail_ = nullptr;
}
......
chrome/browser/ui/thumbnails/thumbnail_image.h
View file @ 7144e845
......@@ -94,12 +94,6 @@ class ThumbnailImage : public base::RefCounted<ThumbnailImage> {
// Observer::OnCompressedThumbnailDataAvailable().
void RequestCompressedThumbnailData();
// Returns the size of the compressed data backing this thumbnail.
// This size can be 0. Additionally, since this data is refcounted,
// it's possible this returns 0 even if the data is still allocated. A
// client can hold a reference to it after |this| drops its reference.
size_t GetCompressedDataSizeInBytes() { return data_->data.size(); }
void set_async_operation_finished_callback_for_testing(
base::RepeatingClosure callback) {
async_operation_finished_callback_ = std::move(callback);
......
chrome/browser/ui/thumbnails/thumbnail_stats_tracker.cc deleted 100644 → 0
View file @ 89a0f3b5
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/ui/thumbnails/thumbnail_stats_tracker.h"
#include "base/bind.h"
#include "base/location.h"
#include "base/metrics/histogram_macros.h"
#include "base/no_destructor.h"
#include "base/time/time.h"
#include "chrome/browser/ui/thumbnails/thumbnail_image.h"
namespace {
// Global instance. Only set once, except in tests which can reset the
// instance and create a new one.
ThumbnailStatsTracker* g_instance = nullptr;
} // namespace
// static
constexpr base::TimeDelta ThumbnailStatsTracker::kReportingInterval;
// static
ThumbnailStatsTracker& ThumbnailStatsTracker::GetInstance() {
if (!g_instance)
g_instance = new ThumbnailStatsTracker;
return *g_instance;
}
// static
void ThumbnailStatsTracker::ResetInstanceForTesting() {
delete g_instance;
g_instance = nullptr;
}
ThumbnailStatsTracker::ThumbnailStatsTracker() {
heartbeat_timer_.Start(
FROM_HERE, kReportingInterval,
base::BindRepeating(&ThumbnailStatsTracker::RecordMetrics,
base::Unretained(this)));
}
ThumbnailStatsTracker::~ThumbnailStatsTracker() {
// This is only called from tests. Make sure there are no thumbnails left.
DCHECK_EQ(thumbnails_.size(), 0u);
}
void ThumbnailStatsTracker::AddThumbnail(ThumbnailImage* thumbnail) {
auto result = thumbnails_.insert(thumbnail);
DCHECK(result.second) << "Thumbnail already added";
}
void ThumbnailStatsTracker::RemoveThumbnail(ThumbnailImage* thumbnail) {
int removed = thumbnails_.erase(thumbnail);
DCHECK_EQ(removed, 1) << "Thumbnail not added";
}
void ThumbnailStatsTracker::RecordMetrics() {
size_t total_size_bytes = 0;
for (ThumbnailImage* thumbnail : thumbnails_) {
size_t thumbnail_size_bytes = thumbnail->GetCompressedDataSizeInBytes();
total_size_bytes += thumbnail_size_bytes;
size_t thumbnail_size_kb = thumbnail_size_bytes / 1024;
UMA_HISTOGRAM_COUNTS_100(
"Tab.Preview.MemoryUsage.CompressedData.PerThumbnailKiB",
thumbnail_size_kb);
}
size_t total_size_kb = total_size_bytes / 1024;
UMA_HISTOGRAM_CUSTOM_COUNTS("Tab.Preview.MemoryUsage.CompressedData.TotalKiB",
total_size_kb, 32, 8192, 100);
}
chrome/browser/ui/thumbnails/thumbnail_stats_tracker.h deleted 100644 → 0
View file @ 89a0f3b5
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROME_BROWSER_UI_THUMBNAILS_THUMBNAIL_STATS_TRACKER_H_
#define CHROME_BROWSER_UI_THUMBNAILS_THUMBNAIL_STATS_TRACKER_H_
#include <set>
#include "base/time/time.h"
#include "base/timer/timer.h"
namespace base {
template <typename T>
class NoDestructor;
} // namespace base
class ThumbnailImage;
// Records memory metrics across all thumbnails in a browser process.
class ThumbnailStatsTracker {
private:
friend class ThumbnailImage;
friend class base::NoDestructor<ThumbnailStatsTracker>;
friend class ThumbnailStatsTrackerTest;
static constexpr base::TimeDelta kReportingInterval =
base::TimeDelta::FromMinutes(5);
// Gets the global instance for this process.
static ThumbnailStatsTracker& GetInstance();
// This must only be called if all registered thumbnails have been
// removed.
static void ResetInstanceForTesting();
ThumbnailStatsTracker();
// Exists only for ResetInstanceForTesting().
~ThumbnailStatsTracker();
// Called from our friend, ThumbnailImage.
void AddThumbnail(ThumbnailImage* thumbnail);
void RemoveThumbnail(ThumbnailImage* thumbnail);
// Called by |heartbeat_timer_| to record metrics at a regular
// interval.
void RecordMetrics();
base::RepeatingTimer heartbeat_timer_;
std::set<ThumbnailImage*> thumbnails_;
};
#endif // CHROME_BROWSER_UI_THUMBNAILS_THUMBNAIL_STATS_TRACKER_H_
chrome/browser/ui/thumbnails/thumbnail_stats_tracker_unittest.cc deleted 100644 → 0
View file @ 89a0f3b5
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/ui/thumbnails/thumbnail_stats_tracker.h"
#include "base/logging.h"
#include "base/memory/scoped_refptr.h"
#include "base/rand_util.h"
#include "base/run_loop.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/task_environment.h"
#include "base/time/time.h"
#include "chrome/browser/ui/thumbnails/thumbnail_image.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/skia/include/core/SkBitmap.h"
namespace {
constexpr char kPerThumbnailMemoryUsageHistogram[] =
"Tab.Preview.MemoryUsage.CompressedData.PerThumbnailKiB";
constexpr char kTotalMemoryUsageHistogram[] =
"Tab.Preview.MemoryUsage.CompressedData.TotalKiB";
class StubThumbnailImageDelegate : public ThumbnailImage::Delegate {
public:
StubThumbnailImageDelegate() = default;
~StubThumbnailImageDelegate() override = default;
// ThumbnailImage::Delegate:
void ThumbnailImageBeingObservedChanged(bool is_being_observed) override {}
};
class ThumbnailOwner : public ThumbnailImage::Delegate {
public:
ThumbnailOwner() = default;
~ThumbnailOwner() override = default;
ThumbnailImage* Get() { return thumbnail_.get(); }
// ThumbnailImage::Delegate:
void ThumbnailImageBeingObservedChanged(bool is_being_observed) override {}
private:
scoped_refptr<ThumbnailImage> thumbnail_{
base::MakeRefCounted<ThumbnailImage>(this)};
};
} // namespace
class ThumbnailStatsTrackerTest : public ::testing::Test {
protected:
void SetUp() override {
// Delete previous instance and create new one to start the timer.
ThumbnailStatsTracker::ResetInstanceForTesting();
ThumbnailStatsTracker::GetInstance();
}
// A random bitmap will have roughly the same size, or even greater
// size, after compression.
SkBitmap CreateRandomBitmapOfSize(unsigned int width, unsigned int height) {
SkBitmap bitmap;
bitmap.allocN32Pixels(width, height);
for (unsigned int x = 0; x < width; ++x) {
for (unsigned int y = 0; y < height; ++y) {
*bitmap.getAddr32(x, y) = static_cast<uint32_t>(base::RandUint64());
}
}
return bitmap;
}
void AssignThumbnailBitmapAndWait(ThumbnailImage* thumbnail,
SkBitmap bitmap) {
base::RunLoop run_loop;
thumbnail->set_async_operation_finished_callback_for_testing(
run_loop.QuitClosure());
thumbnail->AssignSkBitmap(bitmap);
run_loop.Run();
// Clear the callback since the old one will be invalid.
thumbnail->set_async_operation_finished_callback_for_testing(
base::RepeatingClosure());
}
void AdvanceToNextReport() {
task_environment_.FastForwardBy(ThumbnailStatsTracker::kReportingInterval);
}
base::test::TaskEnvironment task_environment_{
base::test::TaskEnvironment::TimeSource::MOCK_TIME};
base::HistogramTester histogram_tester_;
private:
StubThumbnailImageDelegate stub_thumbnail_image_delegate_;
};
TEST_F(ThumbnailStatsTrackerTest, LogsMemoryMetricsAtHeartbeat) {
ThumbnailOwner thumbnail_1;
ThumbnailOwner thumbnail_2;
AssignThumbnailBitmapAndWait(thumbnail_1.Get(),
CreateRandomBitmapOfSize(2, 2));
AssignThumbnailBitmapAndWait(thumbnail_2.Get(),
CreateRandomBitmapOfSize(2, 2));
histogram_tester_.ExpectTotalCount(kPerThumbnailMemoryUsageHistogram, 0);
histogram_tester_.ExpectTotalCount(kTotalMemoryUsageHistogram, 0);
AdvanceToNextReport();
histogram_tester_.ExpectTotalCount(kPerThumbnailMemoryUsageHistogram, 2);
histogram_tester_.ExpectTotalCount(kTotalMemoryUsageHistogram, 1);
AdvanceToNextReport();
histogram_tester_.ExpectTotalCount(kPerThumbnailMemoryUsageHistogram, 4);
histogram_tester_.ExpectTotalCount(kTotalMemoryUsageHistogram, 2);
}
TEST_F(ThumbnailStatsTrackerTest, AlwaysLogsTotal) {
histogram_tester_.ExpectTotalCount(kPerThumbnailMemoryUsageHistogram, 0);
histogram_tester_.ExpectUniqueSample(kTotalMemoryUsageHistogram, 0, 0);
AdvanceToNextReport();
histogram_tester_.ExpectTotalCount(kPerThumbnailMemoryUsageHistogram, 0);
histogram_tester_.ExpectUniqueSample(kTotalMemoryUsageHistogram, 0, 1);
AdvanceToNextReport();
histogram_tester_.ExpectTotalCount(kPerThumbnailMemoryUsageHistogram, 0);
histogram_tester_.ExpectUniqueSample(kTotalMemoryUsageHistogram, 0, 2);
}
TEST_F(ThumbnailStatsTrackerTest, RecordedMemoryUsageIsCorrect) {
ThumbnailOwner thumbnail_1;
AssignThumbnailBitmapAndWait(thumbnail_1.Get(),
CreateRandomBitmapOfSize(200, 150));
size_t thumbnail_1_size_kb =
thumbnail_1.Get()->GetCompressedDataSizeInBytes() / 1024;
AdvanceToNextReport();
histogram_tester_.ExpectBucketCount(kPerThumbnailMemoryUsageHistogram,
thumbnail_1_size_kb, 1);
histogram_tester_.ExpectBucketCount(kTotalMemoryUsageHistogram,
thumbnail_1_size_kb, 1);
ThumbnailOwner thumbnail_2;
AssignThumbnailBitmapAndWait(thumbnail_2.Get(),
CreateRandomBitmapOfSize(100, 100));
size_t thumbnail_2_size_kb =
thumbnail_2.Get()->GetCompressedDataSizeInBytes() / 1024;
// This test won't work if the sizes are the same. While it's possible
// that the two randomly generated bitmaps will compress to the same
// size, the odds are astronomically low.
ASSERT_NE(thumbnail_1_size_kb, thumbnail_2_size_kb);
AdvanceToNextReport();
histogram_tester_.ExpectBucketCount(kPerThumbnailMemoryUsageHistogram,
thumbnail_1_size_kb, 2);
histogram_tester_.ExpectBucketCount(kPerThumbnailMemoryUsageHistogram,
thumbnail_2_size_kb, 1);
histogram_tester_.ExpectBucketCount(kTotalMemoryUsageHistogram,
thumbnail_1_size_kb, 1);
histogram_tester_.ExpectBucketCount(
kTotalMemoryUsageHistogram, thumbnail_1_size_kb + thumbnail_2_size_kb, 1);
}
chrome/test/BUILD.gn
View file @ 7144e845
......@@ -4546,7 +4546,6 @@ test("unit_tests") {
"../browser/ui/thumbnails/thumbnail_capture_driver_unittest.cc",
"../browser/ui/thumbnails/thumbnail_image_unittest.cc",
"../browser/ui/thumbnails/thumbnail_scheduler_impl_unittest.cc",
"../browser/ui/thumbnails/thumbnail_stats_tracker_unittest.cc",
"../browser/ui/toolbar/app_menu_model_unittest.cc",
"../browser/ui/toolbar/back_forward_menu_model_unittest.cc",
"../browser/ui/toolbar/location_bar_model_unittest.cc",
......
tools/metrics/histograms/histograms_xml/tab/histograms.xml
View file @ 7144e845
......@@ -159,27 +159,6 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
<summary>Records how a tab preview image was generated.</summary>
</histogram>
<histogram name="Tab.Preview.MemoryUsage.CompressedData.PerThumbnailKiB"
units="KB" expires_after="M92">
<owner>dfried@chromium.org</owner>
<owner>collinbaker@chromium.org</owner>
<summary>
Memory used by compressed thumbnails. Recorded once per thumbnail every 5
minutes.
</summary>
</histogram>
<histogram name="Tab.Preview.MemoryUsage.CompressedData.TotalKiB" units="KB"
expires_after="M92">
<owner>dfried@chromium.org</owner>
<owner>collinbaker@chromium.org</owner>
<summary>
Memory used by compressed thumbnails. Recorded once every 5 minutes at the
same time as Tab.Preview.MemoryUsage.CompressedData.PerThumbnailKiB. Each
sample is the sum of the corresponding samples in PerThumbnailKiB.
</summary>
</histogram>
<histogram name="Tab.Preview.TimeToFirstUsableFrameAfterStartCapture"
units="ms" expires_after="2020-12-15">
<owner>dfried@chromium.org</owner>
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment