Commit dfc73025 authored by sclittle's avatar sclittle Committed by Commit bot

Changed uses of deque to vector in chrome/browser/android/data_usage.

chrome/browser/android/data_usage has a few uses of std::deque that
would work just about as well using std::vector, and since std::deque is
memory inefficient for small numbers of elements (see
http://crbug.com/674287), this CL replaces those uses of std::deque with
std::vector.

BUG=679568

Review-Url: https://codereview.chromium.org/2624663002
Cr-Commit-Position: refs/heads/master@{#443066}
parent e8e6d1fc
...@@ -134,7 +134,7 @@ void ExternalDataUseObserver::OnDataUse(const data_usage::DataUse& data_use) { ...@@ -134,7 +134,7 @@ void ExternalDataUseObserver::OnDataUse(const data_usage::DataUse& data_use) {
DCHECK(registered_as_data_use_observer_); DCHECK(registered_as_data_use_observer_);
if (!data_use_list_) { if (!data_use_list_) {
data_use_list_.reset(new std::deque<const data_usage::DataUse>()); data_use_list_.reset(new std::vector<const data_usage::DataUse>());
// Post a task to the same IO thread, that will get invoked when some of the // Post a task to the same IO thread, that will get invoked when some of the
// data use objects are batched. // data use objects are batched.
io_task_runner_->PostTask( io_task_runner_->PostTask(
......
...@@ -5,8 +5,8 @@ ...@@ -5,8 +5,8 @@
#ifndef CHROME_BROWSER_ANDROID_DATA_USAGE_EXTERNAL_DATA_USE_OBSERVER_H_ #ifndef CHROME_BROWSER_ANDROID_DATA_USAGE_EXTERNAL_DATA_USE_OBSERVER_H_
#define CHROME_BROWSER_ANDROID_DATA_USAGE_EXTERNAL_DATA_USE_OBSERVER_H_ #define CHROME_BROWSER_ANDROID_DATA_USAGE_EXTERNAL_DATA_USE_OBSERVER_H_
#include <deque>
#include <memory> #include <memory>
#include <vector>
#include "base/gtest_prod_util.h" #include "base/gtest_prod_util.h"
#include "base/macros.h" #include "base/macros.h"
...@@ -116,7 +116,7 @@ class ExternalDataUseObserver : public data_usage::DataUseAggregator::Observer { ...@@ -116,7 +116,7 @@ class ExternalDataUseObserver : public data_usage::DataUseAggregator::Observer {
// Batches the data use objects reported by DataUseAggregator. This will be // Batches the data use objects reported by DataUseAggregator. This will be
// created when data use batching starts and released when the batching ends. // created when data use batching starts and released when the batching ends.
// This will be null if there is no ongoing batching of data use objects. // This will be null if there is no ongoing batching of data use objects.
std::unique_ptr<std::deque<const data_usage::DataUse>> data_use_list_; std::unique_ptr<std::vector<const data_usage::DataUse>> data_use_list_;
// |io_task_runner_| is used to call methods on IO thread. // |io_task_runner_| is used to call methods on IO thread.
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_; scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
......
...@@ -131,7 +131,7 @@ ExternalDataUseReporter::~ExternalDataUseReporter() { ...@@ -131,7 +131,7 @@ ExternalDataUseReporter::~ExternalDataUseReporter() {
} }
void ExternalDataUseReporter::OnDataUse( void ExternalDataUseReporter::OnDataUse(
std::unique_ptr<const std::deque<const data_usage::DataUse>> std::unique_ptr<const std::vector<const data_usage::DataUse>>
data_use_list) { data_use_list) {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(thread_checker_.CalledOnValidThread());
DCHECK(data_use_list); DCHECK(data_use_list);
......
...@@ -8,9 +8,9 @@ ...@@ -8,9 +8,9 @@
#include <stddef.h> #include <stddef.h>
#include <stdint.h> #include <stdint.h>
#include <deque>
#include <memory> #include <memory>
#include <string> #include <string>
#include <vector>
#include "base/callback.h" #include "base/callback.h"
#include "base/containers/hash_tables.h" #include "base/containers/hash_tables.h"
...@@ -86,7 +86,7 @@ class ExternalDataUseReporter { ...@@ -86,7 +86,7 @@ class ExternalDataUseReporter {
// Notifies the ExternalDataUseReporter of data usage. The data use is labeled // Notifies the ExternalDataUseReporter of data usage. The data use is labeled
// using |data_use_tab_model_|, buffered and then reported to // using |data_use_tab_model_|, buffered and then reported to
// |external_data_use_observer_bridge_| later. // |external_data_use_observer_bridge_| later.
void OnDataUse(std::unique_ptr<const std::deque<const data_usage::DataUse>> void OnDataUse(std::unique_ptr<const std::vector<const data_usage::DataUse>>
data_use_list); data_use_list);
void OnReportDataUseDone(bool success); void OnReportDataUseDone(bool success);
......
...@@ -127,8 +127,8 @@ class ExternalDataUseReporterTest : public testing::Test { ...@@ -127,8 +127,8 @@ class ExternalDataUseReporterTest : public testing::Test {
} }
void OnDataUse(const data_usage::DataUse& data_use) { void OnDataUse(const data_usage::DataUse& data_use) {
std::unique_ptr<std::deque<const data_usage::DataUse>> data_use_list( std::unique_ptr<std::vector<const data_usage::DataUse>> data_use_list(
new std::deque<const data_usage::DataUse>()); new std::vector<const data_usage::DataUse>());
data_use_list->push_back(data_use); data_use_list->push_back(data_use);
external_data_use_reporter()->OnDataUse(std::move(data_use_list)); external_data_use_reporter()->OnDataUse(std::move(data_use_list));
} }
......
...@@ -129,7 +129,7 @@ bool TabDataUseEntry::IsTrackingDataUse() const { ...@@ -129,7 +129,7 @@ bool TabDataUseEntry::IsTrackingDataUse() const {
return back_iterator->end_time.is_null(); return back_iterator->end_time.is_null();
} }
const base::TimeTicks TabDataUseEntry::GetLatestStartOrEndTime() const { base::TimeTicks TabDataUseEntry::GetLatestStartOrEndTime() const {
TabSessions::const_reverse_iterator back_iterator = sessions_.rbegin(); TabSessions::const_reverse_iterator back_iterator = sessions_.rbegin();
if (back_iterator == sessions_.rend()) if (back_iterator == sessions_.rend())
return base::TimeTicks(); // No tab session found. return base::TimeTicks(); // No tab session found.
...@@ -140,7 +140,7 @@ const base::TimeTicks TabDataUseEntry::GetLatestStartOrEndTime() const { ...@@ -140,7 +140,7 @@ const base::TimeTicks TabDataUseEntry::GetLatestStartOrEndTime() const {
return back_iterator->start_time; return back_iterator->start_time;
} }
const std::string TabDataUseEntry::GetActiveTrackingSessionLabel() const { std::string TabDataUseEntry::GetActiveTrackingSessionLabel() const {
TabSessions::const_reverse_iterator back_iterator = sessions_.rbegin(); TabSessions::const_reverse_iterator back_iterator = sessions_.rbegin();
if (back_iterator == sessions_.rend() || !IsTrackingDataUse()) if (back_iterator == sessions_.rend() || !IsTrackingDataUse())
return std::string(); return std::string();
...@@ -155,16 +155,22 @@ void TabDataUseEntry::set_custom_tab_package_match( ...@@ -155,16 +155,22 @@ void TabDataUseEntry::set_custom_tab_package_match(
} }
void TabDataUseEntry::CompactSessionHistory() { void TabDataUseEntry::CompactSessionHistory() {
while (sessions_.size() > tab_model_->max_sessions_per_tab()) { if (sessions_.size() <= tab_model_->max_sessions_per_tab())
const auto& front = sessions_.front(); return;
DCHECK(!front.end_time.is_null());
const auto end_it = sessions_.begin() +
(sessions_.size() - tab_model_->max_sessions_per_tab());
for (auto it = sessions_.begin(); it != end_it; ++it) {
DCHECK(!it->end_time.is_null());
// Track how often old sessions are lost. // Track how often old sessions are lost.
UMA_HISTOGRAM_CUSTOM_TIMES(kUMAOldInactiveSessionRemovalDurationHistogram, UMA_HISTOGRAM_CUSTOM_TIMES(kUMAOldInactiveSessionRemovalDurationHistogram,
tab_model_->NowTicks() - front.end_time, tab_model_->NowTicks() - it->end_time,
base::TimeDelta::FromSeconds(1), base::TimeDelta::FromSeconds(1),
base::TimeDelta::FromHours(1), 50); base::TimeDelta::FromHours(1), 50);
sessions_.pop_front();
} }
sessions_.erase(sessions_.begin(), end_it);
} }
} // namespace android } // namespace android
......
...@@ -5,8 +5,8 @@ ...@@ -5,8 +5,8 @@
#ifndef CHROME_BROWSER_ANDROID_DATA_USAGE_TAB_DATA_USE_ENTRY_H_ #ifndef CHROME_BROWSER_ANDROID_DATA_USAGE_TAB_DATA_USE_ENTRY_H_
#define CHROME_BROWSER_ANDROID_DATA_USAGE_TAB_DATA_USE_ENTRY_H_ #define CHROME_BROWSER_ANDROID_DATA_USAGE_TAB_DATA_USE_ENTRY_H_
#include <deque>
#include <string> #include <string>
#include <vector>
#include "base/gtest_prod_util.h" #include "base/gtest_prod_util.h"
#include "base/macros.h" #include "base/macros.h"
...@@ -26,10 +26,10 @@ struct TabDataUseTrackingSession { ...@@ -26,10 +26,10 @@ struct TabDataUseTrackingSession {
: label(label), start_time(start_time) {} : label(label), start_time(start_time) {}
// Tracking label to be associated with the data usage of this session. // Tracking label to be associated with the data usage of this session.
const std::string label; std::string label;
// Time the data use tracking session started. // Time the data use tracking session started.
const base::TimeTicks start_time; base::TimeTicks start_time;
// Time the data use tracking session ended. |end_time| will be null if the // Time the data use tracking session ended. |end_time| will be null if the
// tracking session is currently active. // tracking session is currently active.
...@@ -81,11 +81,11 @@ class TabDataUseEntry { ...@@ -81,11 +81,11 @@ class TabDataUseEntry {
// Returns the latest time a tracking session was started or ended. Returned // Returns the latest time a tracking session was started or ended. Returned
// time will be null if no tracking session was ever started or ended. // time will be null if no tracking session was ever started or ended.
const base::TimeTicks GetLatestStartOrEndTime() const; base::TimeTicks GetLatestStartOrEndTime() const;
// Returns the tracking label for the active tracking session. Empty string is // Returns the tracking label for the active tracking session. Empty string is
// returned if tracking session is not active. // returned if tracking session is not active.
const std::string GetActiveTrackingSessionLabel() const; std::string GetActiveTrackingSessionLabel() const;
bool is_custom_tab_package_match() const { bool is_custom_tab_package_match() const {
return is_custom_tab_package_match_; return is_custom_tab_package_match_;
...@@ -101,7 +101,11 @@ class TabDataUseEntry { ...@@ -101,7 +101,11 @@ class TabDataUseEntry {
ExpiredInactiveTabEntryRemovaltimeHistogram); ExpiredInactiveTabEntryRemovaltimeHistogram);
FRIEND_TEST_ALL_PREFIXES(DataUseTabModelTest, TabCloseEvent); FRIEND_TEST_ALL_PREFIXES(DataUseTabModelTest, TabCloseEvent);
typedef std::deque<TabDataUseTrackingSession> TabSessions; // This is a std::vector instead of a std::deque because std::deque is
// inefficient for small numbers of elements (see crbug/674287). By default,
// at most 5 sessions are tracked at a time per tab, so erasing from the front
// of the vector is cheap anyways.
typedef std::vector<TabDataUseTrackingSession> TabSessions;
// Compacts the history of tracking sessions by removing oldest sessions to // Compacts the history of tracking sessions by removing oldest sessions to
// keep the size of |sessions_| within |kMaxSessionsPerTab| entries. // keep the size of |sessions_| within |kMaxSessionsPerTab| entries.
......
...@@ -163,7 +163,7 @@ class TabDataUseEntryTest : public testing::Test { ...@@ -163,7 +163,7 @@ class TabDataUseEntryTest : public testing::Test {
}; };
// Starts a single tracking session and checks if a new active session is added // Starts a single tracking session and checks if a new active session is added
// to the deque. Ends the session and checks if it becomes inactive. // to the vector. Ends the session and checks if it becomes inactive.
TEST_F(TabDataUseEntryTest, SingleTabSessionStartEnd) { TEST_F(TabDataUseEntryTest, SingleTabSessionStartEnd) {
ExpectTabEntrySessionsSize(TabEntrySessionSize::ZERO); ExpectTabEntrySessionsSize(TabEntrySessionSize::ZERO);
EXPECT_FALSE(tab_entry_->IsTrackingDataUse()); EXPECT_FALSE(tab_entry_->IsTrackingDataUse());
...@@ -178,7 +178,7 @@ TEST_F(TabDataUseEntryTest, SingleTabSessionStartEnd) { ...@@ -178,7 +178,7 @@ TEST_F(TabDataUseEntryTest, SingleTabSessionStartEnd) {
} }
// Starts multiple tracking sessions and checks if new active sessions are added // Starts multiple tracking sessions and checks if new active sessions are added
// to the deque for each. Ends the sessions and checks if they become inactive. // to the vector for each. Ends the sessions and checks if they become inactive.
TEST_F(TabDataUseEntryTest, MultipleTabSessionStartEnd) { TEST_F(TabDataUseEntryTest, MultipleTabSessionStartEnd) {
ExpectTabEntrySessionsSize(TabEntrySessionSize::ZERO); ExpectTabEntrySessionsSize(TabEntrySessionSize::ZERO);
EXPECT_FALSE(tab_entry_->IsTrackingDataUse()); EXPECT_FALSE(tab_entry_->IsTrackingDataUse());
......
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