Skip to content

Commit

Permalink
Re-use callback objects in the DataUseAggregator.
Browse files Browse the repository at this point in the history
This CL makes the DataUseAggregator re-use the same callback objects
separately for annotation and amortization calls, as an optimization to
avoid allocating and holding on to a new callback object for every call.
This CL should have no noticible effects other than using less memory.

BUG=

Review URL: https://codereview.chromium.org/1456793003

Cr-Commit-Position: refs/heads/master@{#360437}
  • Loading branch information
scott-little authored and Commit bot committed Nov 18, 2015
1 parent c9e3b72 commit 2955091
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 14 deletions.
32 changes: 20 additions & 12 deletions components/data_usage/core/data_use_aggregator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
#include "base/callback.h"
#include "base/stl_util.h"
#include "components/data_usage/core/data_use.h"
#include "components/data_usage/core/data_use_amortizer.h"
#include "components/data_usage/core/data_use_annotator.h"
#include "net/base/load_timing_info.h"
#include "net/base/network_change_notifier.h"
#include "net/url_request/url_request.h"
Expand All @@ -34,6 +32,10 @@ DataUseAggregator::DataUseAggregator(scoped_ptr<DataUseAnnotator> annotator,

DataUseAggregator::~DataUseAggregator() {
net::NetworkChangeNotifier::RemoveConnectionTypeObserver(this);

// Reset the callbacks to remove any WeakPtr references to |this| inside them.
annotation_callback_.Reset();
amortization_callback_.Reset();
}

void DataUseAggregator::AddObserver(Observer* observer) {
Expand Down Expand Up @@ -64,11 +66,14 @@ void DataUseAggregator::ReportDataUse(net::URLRequest* request,
return;
}

// TODO(sclittle): Instead of binding a new callback every time, re-use the
// same callback every time.
annotator_->Annotate(
request, data_use.Pass(),
base::Bind(&DataUseAggregator::PassDataUseToAmortizer, GetWeakPtr()));
// As an optimization, re-use a lazily initialized callback object for every
// call into |annotator_|, so that a new callback object doesn't have to be
// allocated and held onto every time.
if (annotation_callback_.is_null()) {
annotation_callback_ =
base::Bind(&DataUseAggregator::PassDataUseToAmortizer, GetWeakPtr());
}
annotator_->Annotate(request, data_use.Pass(), annotation_callback_);
}

void DataUseAggregator::ReportOffTheRecordDataUse(int64_t tx_bytes,
Expand Down Expand Up @@ -109,11 +114,14 @@ void DataUseAggregator::PassDataUseToAmortizer(scoped_ptr<DataUse> data_use) {
return;
}

// TODO(sclittle): Instead of binding a new callback every time, re-use the
// same callback every time.
amortizer_->AmortizeDataUse(
data_use.Pass(),
base::Bind(&DataUseAggregator::OnAmortizationComplete, GetWeakPtr()));
// As an optimization, re-use a lazily initialized callback object for every
// call into |amortizer_|, so that a new callback object doesn't have to be
// allocated and held onto every time.
if (amortization_callback_.is_null()) {
amortization_callback_ =
base::Bind(&DataUseAggregator::OnAmortizationComplete, GetWeakPtr());
}
amortizer_->AmortizeDataUse(data_use.Pass(), amortization_callback_);
}

void DataUseAggregator::OnAmortizationComplete(
Expand Down
10 changes: 8 additions & 2 deletions components/data_usage/core/data_use_aggregator.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#include "base/memory/weak_ptr.h"
#include "base/observer_list.h"
#include "base/threading/thread_checker.h"
#include "components/data_usage/core/data_use_amortizer.h"
#include "components/data_usage/core/data_use_annotator.h"
#include "net/base/network_change_notifier.h"

namespace net {
Expand All @@ -23,8 +25,6 @@ class URLRequest;

namespace data_usage {

class DataUseAmortizer;
class DataUseAnnotator;
struct DataUse;

// Class that collects and aggregates network usage, reporting the usage to
Expand Down Expand Up @@ -96,6 +96,12 @@ class DataUseAggregator
// even if the current active network is not a cellular network.
std::string mcc_mnc_;

// As an optimization, re-use the same callbacks to avoid creating and
// allocating a new Callback object for each call into the |annotator_| or
// |amortizer_|. These callbacks are lazily initialized.
DataUseAnnotator::DataUseConsumerCallback annotation_callback_;
DataUseAmortizer::AmortizationCompleteCallback amortization_callback_;

base::WeakPtrFactory<DataUseAggregator> weak_ptr_factory_;

DISALLOW_COPY_AND_ASSIGN(DataUseAggregator);
Expand Down

0 comments on commit 2955091

Please sign in to comment.