Commit 6765f04b authored by Lubo Ivanov's avatar Lubo Ivanov Committed by Chromium LUCI CQ

perfetto: Enable batching of commits and producer-side chunk patching in

Chrome's perfetto producers.

This is done to reduce the overhead of tracing.

The batching period of 1s was determined using
https://chromium-review.googlesource.com/c/chromium/src/+/1835498 in
Chrome on gLinux, but with 4KB Chunk size (which corresponds to the
chunk size on Android, where batching would have the most pronounced
effect). The effects of various delays on overhead can be seen at
https://drive.google.com/file/d/1Og9cSTaNmsSy1P6k0nfoyHK8n094HMrW/view?usp=sharing.
A spreadsheet with the full perfstat results, on which this graph is
based, can be found at
https://docs.google.com/spreadsheets/d/15wxYJaR2hvUVY2MtqI29gPYwdkGxDZVqrUkSOZWspPs/edit?usp=sharing.


Since all affected producers already set will_notify_on_stop=true, we
can just flush commits while stopping a datasource to avoid losing any
batched data. Related changes in perfetto:

https://android-review.googlesource.com/c/platform/external/perfetto/+/1260461
https://android-review.googlesource.com/c/platform/external/perfetto/+/1309414/13
https://android-review.googlesource.com/c/platform/external/perfetto/+/1450696/14

Bug: 1029298 Change-Id: I880cb2e49c169bcf6e216eb12ce52b8bd40ba4ad
Change-Id: I880cb2e49c169bcf6e216eb12ce52b8bd40ba4ad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2149648
Commit-Queue: Lyubomir Ivanov <lri@google.com>
Reviewed-by: default avatarSami Kyöstilä <skyostil@chromium.org>
Reviewed-by: default avatarEric Seckler <eseckler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837109}
parent f723a230
...@@ -122,6 +122,22 @@ class COMPONENT_EXPORT(TRACING_CPP) PerfettoProducer { ...@@ -122,6 +122,22 @@ class COMPONENT_EXPORT(TRACING_CPP) PerfettoProducer {
// TODO(crbug.com/839071): Figure out a good buffer size. // TODO(crbug.com/839071): Figure out a good buffer size.
static constexpr size_t kSMBSizeBytes = 4 * 1024 * 1024; static constexpr size_t kSMBSizeBytes = 4 * 1024 * 1024;
// TODO(lri): replace this constant with its version in the client library,
// when we move over.
//
// This value for SharedMemoryArbiter's batch_commits_duration_ms was
// determined by load testing, using the script at
// https://chromium-review.googlesource.com/c/chromium/src/+/1835498. The
// effects of various delays on the overhead of tracing in Chrome
// can be seen at https://screenshot.googleplex.com/KgsJshNCFKq. See commit
// 2fc0474d9 and crbug.com/1029298 for more context.
//
// Note that since this value is non-zero, it could lead to loss of batched
// data at the end of a tracing session. The producer should enable
// asynchronous stopping of datasources and should flush the accumulated
// commits while a datasource is being stopped.
static constexpr uint32_t kShmArbiterBatchCommitDurationMs = 1000;
PerfettoTaskRunner* task_runner(); PerfettoTaskRunner* task_runner();
SEQUENCE_CHECKER(sequence_checker_); SEQUENCE_CHECKER(sequence_checker_);
......
...@@ -243,6 +243,11 @@ void PosixSystemProducer::OnTracingSetup() { ...@@ -243,6 +243,11 @@ void PosixSystemProducer::OnTracingSetup() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Called by the IPC layer when tracing is first started and after shared // Called by the IPC layer when tracing is first started and after shared
// memory is set up. // memory is set up.
DCHECK(MaybeSharedMemoryArbiter());
if (MaybeSharedMemoryArbiter()->EnableDirectSMBPatching()) {
MaybeSharedMemoryArbiter()->SetBatchCommitsDuration(
kShmArbiterBatchCommitDurationMs);
}
} }
void PosixSystemProducer::SetupDataSource(perfetto::DataSourceInstanceID, void PosixSystemProducer::SetupDataSource(perfetto::DataSourceInstanceID,
...@@ -306,6 +311,11 @@ void PosixSystemProducer::StopDataSource(perfetto::DataSourceInstanceID id) { ...@@ -306,6 +311,11 @@ void PosixSystemProducer::StopDataSource(perfetto::DataSourceInstanceID id) {
return; return;
} }
DCHECK_CALLED_ON_VALID_SEQUENCE(weak_ptr->sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(weak_ptr->sequence_checker_);
// Flush any commits that might have been batched by
// SharedMemoryArbiter.
weak_ptr->GetService()
->MaybeSharedMemoryArbiter()
->FlushPendingCommitDataRequests();
weak_ptr->GetService()->NotifyDataSourceStopped(id); weak_ptr->GetService()->NotifyDataSourceStopped(id);
{ {
base::AutoLock lock(weak_ptr->lock_); base::AutoLock lock(weak_ptr->lock_);
......
...@@ -238,10 +238,16 @@ void ProducerClient::StopDataSource(uint64_t id, ...@@ -238,10 +238,16 @@ void ProducerClient::StopDataSource(uint64_t id,
data_source->StopTracing(base::BindOnce( data_source->StopTracing(base::BindOnce(
[](base::WeakPtr<ProducerClient> weak_ptr, [](base::WeakPtr<ProducerClient> weak_ptr,
StopDataSourceCallback callback, uint64_t id) { StopDataSourceCallback callback, uint64_t id) {
std::move(callback).Run(); if (!weak_ptr) {
if (!weak_ptr) std::move(callback).Run();
return; return;
}
DCHECK_CALLED_ON_VALID_SEQUENCE(weak_ptr->sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(weak_ptr->sequence_checker_);
// Flush any commits that might have been batched by
// SharedMemoryArbiter.
weak_ptr->MaybeSharedMemoryArbiter()
->FlushPendingCommitDataRequests();
std::move(callback).Run();
base::AutoLock lock(weak_ptr->lock_); base::AutoLock lock(weak_ptr->lock_);
--weak_ptr->data_sources_tracing_; --weak_ptr->data_sources_tracing_;
}, },
...@@ -404,6 +410,10 @@ bool ProducerClient::InitSharedMemoryIfNeeded() { ...@@ -404,6 +410,10 @@ bool ProducerClient::InitSharedMemoryIfNeeded() {
shared_memory_arbiter_ = perfetto::SharedMemoryArbiter::CreateUnboundInstance( shared_memory_arbiter_ = perfetto::SharedMemoryArbiter::CreateUnboundInstance(
shared_memory_.get(), kSMBPageSizeBytes); shared_memory_.get(), kSMBPageSizeBytes);
shared_memory_arbiter_->SetDirectSMBPatchingSupportedByService();
shared_memory_arbiter_->EnableDirectSMBPatching();
shared_memory_arbiter_->SetBatchCommitsDuration(
kShmArbiterBatchCommitDurationMs);
return true; return true;
} }
......
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