Skip to content

Commit

Permalink
[ads] Optimize database queries serialization/deserialization overhead
Browse files Browse the repository at this point in the history
  • Loading branch information
aseren committed Dec 4, 2024
1 parent 59a62da commit 047d95e
Show file tree
Hide file tree
Showing 48 changed files with 346 additions and 259 deletions.
23 changes: 1 addition & 22 deletions components/brave_ads/browser/ads_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ void AdsServiceImpl::StartBatAdsService() {
}

bat_ads_service_remote_->Create(
ads_service_path_,
bat_ads_client_associated_receiver_.BindNewEndpointAndPassRemote(),
bat_ads_associated_remote_.BindNewEndpointAndPassReceiver(),
std::move(bat_ads_client_notifier_pending_receiver_),
Expand Down Expand Up @@ -410,18 +411,9 @@ void AdsServiceImpl::Initialize(const size_t current_start_number) {
return;
}

InitializeDatabase();

InitializeRewardsWallet(current_start_number);
}

void AdsServiceImpl::InitializeDatabase() {
CHECK(!database_);

database_ = base::SequenceBound<Database>(
file_task_runner_, ads_service_path_.AppendASCII(kDatabaseFilename));
}

void AdsServiceImpl::InitializeRewardsWallet(
const size_t current_start_number) {
rewards_service_observation_.GetSource()->GetRewardsWallet(
Expand Down Expand Up @@ -1100,8 +1092,6 @@ void AdsServiceImpl::ShutdownAdsService() {

CloseAdaptiveCaptcha();

database_.Reset();

if (is_bat_ads_initialized_) {
VLOG(2) << "Shutdown Bat Ads Service";
}
Expand Down Expand Up @@ -1759,17 +1749,6 @@ void AdsServiceImpl::ShowScheduledCaptcha(const std::string& payment_id,
#endif // !BUILDFLAG(IS_ANDROID)
}

void AdsServiceImpl::RunDBTransaction(
mojom::DBTransactionInfoPtr mojom_db_transaction,
RunDBTransactionCallback callback) {
CHECK(mojom_db_transaction);
CHECK(database_);

database_.AsyncCall(&Database::RunDBTransaction)
.WithArgs(std::move(mojom_db_transaction))
.Then(std::move(callback));
}

void AdsServiceImpl::RecordP2AEvents(const std::vector<std::string>& events) {
for (const auto& event : events) {
RecordAndEmitP2AHistogramName(prefs_,
Expand Down
8 changes: 0 additions & 8 deletions components/brave_ads/browser/ads_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#include "base/memory/scoped_refptr.h"
#include "base/memory/weak_ptr.h"
#include "base/task/cancelable_task_tracker.h"
#include "base/threading/sequence_bound.h"
#include "base/time/time.h"
#include "base/timer/timer.h"
#include "brave/components/brave_adaptive_captcha/brave_adaptive_captcha_service.h"
Expand Down Expand Up @@ -61,7 +60,6 @@ namespace brave_ads {
class AdsServiceObserver;
class AdsTooltipsDelegate;
class BatAdsServiceFactory;
class Database;
class DeviceId;
class ResourceComponent;
struct NewTabPageAdInfo;
Expand Down Expand Up @@ -127,7 +125,6 @@ class AdsServiceImpl final : public AdsService,
void InitializeBasePathDirectoryCallback(size_t current_start_number,
bool success);
void Initialize(size_t current_start_number);
void InitializeDatabase();
void InitializeRewardsWallet(size_t current_start_number);
void InitializeRewardsWalletCallback(
size_t current_start_number,
Expand Down Expand Up @@ -370,9 +367,6 @@ class AdsServiceImpl final : public AdsService,
void ShowScheduledCaptcha(const std::string& payment_id,
const std::string& captcha_id) override;

void RunDBTransaction(mojom::DBTransactionInfoPtr mojom_db_transaction,
RunDBTransactionCallback callback) override;

// TODO(https://github.com/brave/brave-browser/issues/14666) Decouple P2A
// business logic.
void RecordP2AEvents(const std::vector<std::string>& events) override;
Expand Down Expand Up @@ -440,8 +434,6 @@ class AdsServiceImpl final : public AdsService,

mojom::SysInfo sys_info_;

base::SequenceBound<Database> database_;

base::RepeatingTimer idle_state_timer_;
ui::IdleState last_idle_state_ = ui::IdleState::IDLE_STATE_ACTIVE;
base::TimeDelta last_idle_time_;
Expand Down
3 changes: 3 additions & 0 deletions components/brave_ads/core/internal/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,7 @@ static_library("internal") {
"creatives/segments_database_table.h",
"creatives/segments_database_util.cc",
"creatives/segments_database_util.h",
"database/background_database_handler.cc",
"database/database.cc",
"database/database_maintenance.cc",
"database/database_maintenance.h",
Expand All @@ -594,6 +595,8 @@ static_library("internal") {
"database/database_manager_observer.h",
"database/database_table_interface.cc",
"database/database_table_interface.h",
"database/test_database_handler.cc",
"database/test_database_handler.h",
"deprecated/client/client_info.cc",
"deprecated/client/client_info.h",
"deprecated/client/client_state_manager.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include "brave/components/brave_ads/core/internal/account/confirmations/queue/queue_item/confirmation_queue_item_builder_util.h"
#include "brave/components/brave_ads/core/internal/account/confirmations/queue/queue_item/confirmation_queue_item_util.h"
#include "brave/components/brave_ads/core/internal/account/confirmations/reward/reward_confirmation_util.h"
#include "brave/components/brave_ads/core/internal/ads_client/ads_client_util.h"
#include "brave/components/brave_ads/core/internal/common/challenge_bypass_ristretto/blinded_token.h"
#include "brave/components/brave_ads/core/internal/common/challenge_bypass_ristretto/public_key.h"
#include "brave/components/brave_ads/core/internal/common/challenge_bypass_ristretto/token.h"
Expand All @@ -33,7 +32,6 @@
#include "brave/components/brave_ads/core/mojom/brave_ads.mojom.h"
#include "brave/components/brave_ads/core/public/account/confirmations/confirmation_type.h"
#include "brave/components/brave_ads/core/public/ad_units/ad_type.h"
#include "brave/components/brave_ads/core/public/ads_client/ads_client.h"

namespace brave_ads::database::table {

Expand Down Expand Up @@ -396,9 +394,8 @@ void ConfirmationQueue::GetAll(GetConfirmationQueueCallback callback) const {
BindColumnTypes(mojom_db_action);
mojom_db_transaction->actions.push_back(std::move(mojom_db_action));

GetAdsClient().RunDBTransaction(
std::move(mojom_db_transaction),
base::BindOnce(&GetCallback, std::move(callback)));
RunDBTransaction(std::move(mojom_db_transaction),
base::BindOnce(&GetCallback, std::move(callback)));
}

void ConfirmationQueue::GetNext(GetConfirmationQueueCallback callback) const {
Expand Down Expand Up @@ -433,9 +430,8 @@ void ConfirmationQueue::GetNext(GetConfirmationQueueCallback callback) const {
BindColumnTypes(mojom_db_action);
mojom_db_transaction->actions.push_back(std::move(mojom_db_action));

GetAdsClient().RunDBTransaction(
std::move(mojom_db_transaction),
base::BindOnce(&GetCallback, std::move(callback)));
RunDBTransaction(std::move(mojom_db_transaction),
base::BindOnce(&GetCallback, std::move(callback)));
}

std::string ConfirmationQueue::GetTableName() const {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,13 @@
#include "base/check.h"
#include "base/functional/bind.h"
#include "base/strings/string_util.h"
#include "brave/components/brave_ads/core/internal/ads_client/ads_client_util.h"
#include "brave/components/brave_ads/core/internal/common/database/database_column_util.h"
#include "brave/components/brave_ads/core/internal/common/database/database_statement_util.h"
#include "brave/components/brave_ads/core/internal/common/database/database_table_util.h"
#include "brave/components/brave_ads/core/internal/common/database/database_transaction_util.h"
#include "brave/components/brave_ads/core/internal/common/logging_util.h"
#include "brave/components/brave_ads/core/internal/common/time/time_util.h"
#include "brave/components/brave_ads/core/mojom/brave_ads.mojom.h"
#include "brave/components/brave_ads/core/public/ads_client/ads_client.h"

namespace brave_ads::database::table {

Expand Down Expand Up @@ -191,10 +189,9 @@ void Deposits::GetForCreativeInstanceId(const std::string& creative_instance_id,
BindColumnTypes(mojom_db_action);
mojom_db_transaction->actions.push_back(std::move(mojom_db_action));

GetAdsClient().RunDBTransaction(
std::move(mojom_db_transaction),
base::BindOnce(&GetForCreativeInstanceIdCallback, creative_instance_id,
std::move(callback)));
RunDBTransaction(std::move(mojom_db_transaction),
base::BindOnce(&GetForCreativeInstanceIdCallback,
creative_instance_id, std::move(callback)));
}

void Deposits::PurgeExpired(ResultCallback callback) const {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,8 @@ void Transactions::GetForDateRange(const base::Time from_time,
BindColumnTypes(mojom_db_action);
mojom_db_transaction->actions.push_back(std::move(mojom_db_action));

GetAdsClient().RunDBTransaction(
std::move(mojom_db_transaction),
base::BindOnce(&GetCallback, std::move(callback)));
RunDBTransaction(std::move(mojom_db_transaction),
base::BindOnce(&GetCallback, std::move(callback)));
}

void Transactions::Reconcile(const PaymentTokenList& payment_tokens,
Expand Down
10 changes: 8 additions & 2 deletions components/brave_ads/core/internal/ads.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,21 @@

#include "brave/components/brave_ads/core/public/ads.h"

#include <utility>

#include "brave/components/brave_ads/core/internal/account/tokens/token_generator.h"
#include "brave/components/brave_ads/core/internal/ads_impl.h"
#include "brave/components/brave_ads/core/public/database/database_handler_interface.h"

namespace brave_ads {

// static
std::unique_ptr<Ads> Ads::CreateInstance(AdsClient& ads_client) {
std::unique_ptr<Ads> Ads::CreateInstance(
AdsClient& ads_client,
std::unique_ptr<DatabaseHandlerInterface> database_handler) {
return std::make_unique<AdsImpl>(ads_client,
std::make_unique<TokenGenerator>());
std::make_unique<TokenGenerator>(),
std::move(database_handler));
}

} // namespace brave_ads
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,6 @@ class AdsClientMock : public AdsClient {
ShowScheduledCaptcha,
(const std::string& payment_id, const std::string& captcha_id));

MOCK_METHOD(void,
RunDBTransaction,
(mojom::DBTransactionInfoPtr, RunDBTransactionCallback));

MOCK_METHOD(void, RecordP2AEvents, (const std::vector<std::string>& events));

MOCK_METHOD(bool, FindProfilePref, (const std::string& path), (const));
Expand Down
9 changes: 7 additions & 2 deletions components/brave_ads/core/internal/ads_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "brave/components/brave_ads/core/internal/ads_impl.h"

#include <memory>
#include <optional>
#include <utility>

Expand All @@ -26,6 +27,7 @@
#include "brave/components/brave_ads/core/internal/legacy_migration/confirmations/legacy_confirmation_migration.h"
#include "brave/components/brave_ads/core/internal/user_engagement/ad_events/ad_events.h"
#include "brave/components/brave_ads/core/public/ads_client/ads_client.h"
#include "brave/components/brave_ads/core/public/database/database_handler_interface.h"
#include "brave/components/brave_ads/core/public/service/ads_service_callback.h"

namespace brave_ads {
Expand All @@ -41,8 +43,11 @@ void FailedToInitialize(InitializeCallback callback) {
} // namespace

AdsImpl::AdsImpl(AdsClient& ads_client,
std::unique_ptr<TokenGeneratorInterface> token_generator)
: global_state_(ads_client, std::move(token_generator)),
std::unique_ptr<TokenGeneratorInterface> token_generator,
std::unique_ptr<DatabaseHandlerInterface> database_handler)
: global_state_(ads_client,
std::move(token_generator),
std::move(database_handler)),
database_maintenance_(std::make_unique<database::Maintenance>()) {}

AdsImpl::~AdsImpl() = default;
Expand Down
3 changes: 2 additions & 1 deletion components/brave_ads/core/internal/ads_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ class Maintenance;
class AdsImpl final : public Ads {
public:
AdsImpl(AdsClient& ads_client,
std::unique_ptr<TokenGeneratorInterface> token_generator);
std::unique_ptr<TokenGeneratorInterface> token_generator,
std::unique_ptr<DatabaseHandlerInterface> database_handler);

AdsImpl(const AdsImpl&) = delete;
AdsImpl& operator=(const AdsImpl&) = delete;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@

#include "base/functional/bind.h"
#include "base/strings/string_util.h"
#include "brave/components/brave_ads/core/internal/ads_client/ads_client_util.h"
#include "brave/components/brave_ads/core/internal/database/database_manager.h"
#include "brave/components/brave_ads/core/internal/global_state/global_state.h"
#include "brave/components/brave_ads/core/mojom/brave_ads.mojom.h"
#include "brave/components/brave_ads/core/public/ads_client/ads_client.h"
#include "brave/components/brave_ads/core/public/ads_client/ads_client_callback.h"

namespace brave_ads::database {

Expand Down Expand Up @@ -43,9 +44,15 @@ bool IsError(
mojom::DBTransactionResultInfo::StatusCode::kSuccess;
}

void RunDBTransaction(mojom::DBTransactionInfoPtr mojom_db_transaction,
::brave_ads::RunDBTransactionCallback callback) {
GlobalState::GetInstance()->GetDatabaseManager().RunDBTransaction(
std::move(mojom_db_transaction), std::move(callback));
}

void RunDBTransaction(mojom::DBTransactionInfoPtr mojom_db_transaction,
ResultCallback callback) {
GetAdsClient().RunDBTransaction(
GlobalState::GetInstance()->GetDatabaseManager().RunDBTransaction(
std::move(mojom_db_transaction),
base::BindOnce(&RunDBTransactionCallback, std::move(callback)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ bool IsSuccess(
bool IsError(
const mojom::DBTransactionResultInfoPtr& mojom_db_transaction_result);

// Run a database transaction. The callback takes one argument -
// `mojom::DBTransactionResultInfoPtr` containing the info of the transaction.
void RunDBTransaction(mojom::DBTransactionInfoPtr mojom_db_transaction,
RunDBTransactionCallback callback);

// Run a database transaction.
void RunDBTransaction(mojom::DBTransactionInfoPtr mojom_db_transaction,
ResultCallback callback);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,20 +224,6 @@ void MockLoadDataResource(AdsClientMock& ads_client_mock) {
}));
}

void MockRunDBTransaction(AdsClientMock& ads_client_mock, Database& database) {
ON_CALL(ads_client_mock, RunDBTransaction)
.WillByDefault(::testing::Invoke(
[&database](mojom::DBTransactionInfoPtr mojom_db_transaction,
RunDBTransactionCallback callback) {
CHECK(mojom_db_transaction);

mojom::DBTransactionResultInfoPtr mojom_db_transaction_result =
database.RunDBTransaction(std::move(mojom_db_transaction));

std::move(callback).Run(std::move(mojom_db_transaction_result));
}));
}

void MockFindProfilePref(const AdsClientMock& ads_client_mock) {
ON_CALL(ads_client_mock, FindProfilePref)
.WillByDefault(::testing::Invoke([](const std::string& path) -> bool {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ void MockLoadResourceComponent(AdsClientMock& ads_client_mock,

void MockLoadDataResource(AdsClientMock& ads_client_mock);

void MockRunDBTransaction(AdsClientMock& ads_client_mock, Database& database);

void MockFindProfilePref(const AdsClientMock& ads_client_mock);
void MockGetProfilePref(const AdsClientMock& ads_client_mock);
void MockSetProfilePref(const AdsClientMock& ads_client_mock,
Expand Down
14 changes: 7 additions & 7 deletions components/brave_ads/core/internal/common/test/test_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@
#include "brave/components/brave_ads/core/internal/common/test/test_types.h"
#include "brave/components/brave_ads/core/internal/common/test/time_test_util.h"
#include "brave/components/brave_ads/core/internal/database/database_manager.h"
#include "brave/components/brave_ads/core/internal/database/test_database_handler.h"
#include "brave/components/brave_ads/core/internal/deprecated/client/client_state_manager.h"
#include "brave/components/brave_ads/core/internal/deprecated/confirmations/confirmation_state_manager.h"
#include "brave/components/brave_ads/core/internal/global_state/global_state.h"
#include "brave/components/brave_ads/core/public/ads.h"
#include "brave/components/brave_ads/core/public/ads_constants.h"
#include "brave/components/brave_ads/core/public/database/database.h"

namespace brave_ads::test {

Expand Down Expand Up @@ -238,10 +238,6 @@ void TestBase::MockAdsClient() {

MockLoadDataResource(ads_client_mock_);

database_ =
std::make_unique<Database>(ProfilePath().AppendASCII(kDatabaseFilename));
MockRunDBTransaction(ads_client_mock_, *database_);

MockFindProfilePref(ads_client_mock_);
MockGetProfilePref(ads_client_mock_);
MockSetProfilePref(ads_client_mock_, *this);
Expand Down Expand Up @@ -302,7 +298,9 @@ void TestBase::SetUpIntegrationTest() {
<< "SetUpIntegrationTest should only be called if SetUp is initialized "
"for integration testing";

ads_ = Ads::CreateInstance(ads_client_mock_);
ads_ = Ads::CreateInstance(ads_client_mock_,
std::make_unique<TestDatabaseHandler>(
ProfilePath().AppendASCII(kDatabaseFilename)));
CHECK(ads_) << "Failed to create ads instance";

// Must be called after `Ads` is instantiated but prior to `Initialize`.
Expand Down Expand Up @@ -332,7 +330,9 @@ void TestBase::SetUpUnitTest() {
"SetUp is initialized for unit testing";

global_state_ = std::make_unique<GlobalState>(
ads_client_mock_, std::make_unique<TokenGeneratorMock>());
ads_client_mock_, std::make_unique<TokenGeneratorMock>(),
std::make_unique<TestDatabaseHandler>(
ProfilePath().AppendASCII(kDatabaseFilename)));

// Must be called after `GlobalState` is instantiated but prior to
// `MockDefaultAdsServiceState`.
Expand Down
Loading

0 comments on commit 047d95e

Please sign in to comment.