Fix logic to decide SMB page and size from config
The current logic that was deciding the SMB total size
and page size, using both data from the producer hint and
data in the trace config was broken in some corner cases.
This CL fixes the bugs and adds tests.
Bug: 73625175
Test: perfetto_unittests --gtest_filter=ServiceImplTest.*
Change-Id: Id0da9dbe64c2008f6296362ab8057503a5cd5efd
diff --git a/src/tracing/core/service_impl_unittest.cc b/src/tracing/core/service_impl_unittest.cc
index 1ea2534..d166192 100644
--- a/src/tracing/core/service_impl_unittest.cc
+++ b/src/tracing/core/service_impl_unittest.cc
@@ -22,6 +22,7 @@
#include "gtest/gtest.h"
#include "perfetto/base/file_utils.h"
#include "perfetto/base/temp_file.h"
+#include "perfetto/base/utils.h"
#include "perfetto/tracing/core/consumer.h"
#include "perfetto/tracing/core/data_source_config.h"
#include "perfetto/tracing/core/data_source_descriptor.h"
@@ -41,6 +42,7 @@
using ::testing::_;
using ::testing::Contains;
+using ::testing::ElementsAreArray;
using ::testing::Eq;
using ::testing::InSequence;
using ::testing::Invoke;
@@ -51,6 +53,11 @@
namespace perfetto {
+namespace {
+constexpr size_t kDefaultShmSizeKb = ServiceImpl::kDefaultShmSize / 1024;
+constexpr size_t kMaxShmSizeKb = ServiceImpl::kMaxShmSize / 1024;
+} // namespace
+
class ServiceImplTest : public testing::Test {
public:
ServiceImplTest() {
@@ -71,6 +78,12 @@
new StrictMock<MockConsumer>(&task_runner));
}
+ ProducerID* last_producer_id() { return &svc->last_producer_id_; }
+
+ uid_t GetProducerUid(ProducerID producer_id) {
+ return svc->GetProducer(producer_id)->uid_;
+ }
+
base::TestTaskRunner task_runner;
std::unique_ptr<ServiceImpl> svc;
};
@@ -85,8 +98,8 @@
ASSERT_EQ(2u, svc->num_producers());
ASSERT_EQ(mock_producer_1->endpoint(), svc->GetProducer(1));
ASSERT_EQ(mock_producer_2->endpoint(), svc->GetProducer(2));
- ASSERT_EQ(123u, svc->GetProducer(1)->uid_);
- ASSERT_EQ(456u, svc->GetProducer(2)->uid_);
+ ASSERT_EQ(123u, GetProducerUid(1));
+ ASSERT_EQ(456u, GetProducerUid(2));
mock_producer_1->RegisterDataSource("foo");
mock_producer_2->RegisterDataSource("bar");
@@ -228,7 +241,7 @@
this](const std::string& name) {
producers.emplace_back(CreateMockProducer());
producers.back()->Connect(svc.get(), "mock_producer_" + name);
- return svc->last_producer_id_;
+ return *last_producer_id();
};
// Connect producers 1-4.
@@ -239,7 +252,7 @@
producers[1].reset();
producers[3].reset();
- svc->last_producer_id_ = kMaxProducerID - 1;
+ *last_producer_id() = kMaxProducerID - 1;
ASSERT_EQ(kMaxProducerID, connect_producer_and_get_id("maxid"));
ASSERT_EQ(1u, connect_producer_and_get_id("1_again"));
ASSERT_EQ(3u, connect_producer_and_get_id("3_again"));
@@ -313,4 +326,62 @@
}
}
+// Test the logic that allows the trace config to set the shm total size and
+// page size from the trace config. Also check that, if the config doesn't
+// specify a value we fall back on the hint provided by the producer.
+TEST_F(ServiceImplTest, ProducerShmAndPageSizeOverriddenByTraceConfig) {
+ std::unique_ptr<MockConsumer> consumer = CreateMockConsumer();
+ consumer->Connect(svc.get());
+ const size_t kConfigPageSizesKb[] = /****/ {16, 16, 4, 0, 16, 8, 3, 4096, 4};
+ const size_t kExpectedPageSizesKb[] = /**/ {16, 16, 4, 4, 16, 8, 4, 64, 4};
+
+ const size_t kConfigSizesKb[] = /**/ {0, 16, 0, 20, 32, 7, 0, 96, 4096000};
+ const size_t kHintSizesKb[] = /****/ {0, 0, 16, 32, 16, 0, 7, 96, 4096000};
+ const size_t kExpectedSizesKb[] = {
+ kDefaultShmSizeKb, // Both hint and config are 0, use default.
+ 16, // Hint is 0, use config.
+ 16, // Config is 0, use hint.
+ 20, // Hint is takes precedence over the config.
+ 32, // Ditto, even if config is higher than hint.
+ kDefaultShmSizeKb, // Config is invalid and hint is 0, use default.
+ kDefaultShmSizeKb, // Config is 0 and hint is invalid, use default.
+ kDefaultShmSizeKb, // 96 KB isn't a multiple of the page size (64 KB).
+ kMaxShmSizeKb // Too big, cap at kMaxShmSize.
+ };
+
+ const size_t kNumProducers = base::ArraySize(kHintSizesKb);
+ std::unique_ptr<MockProducer> producer[kNumProducers];
+ for (size_t i = 0; i < kNumProducers; i++) {
+ auto name = "mock_producer_" + std::to_string(i);
+ producer[i] = CreateMockProducer();
+ producer[i]->Connect(svc.get(), name, geteuid(), kHintSizesKb[i] * 1024);
+ producer[i]->RegisterDataSource("data_source");
+ }
+
+ TraceConfig trace_config;
+ trace_config.add_buffers()->set_size_kb(128);
+ auto* ds_config = trace_config.add_data_sources()->mutable_config();
+ ds_config->set_name("data_source");
+ for (size_t i = 0; i < kNumProducers; i++) {
+ auto* producer_config = trace_config.add_producers();
+ producer_config->set_producer_name("mock_producer_" + std::to_string(i));
+ producer_config->set_shm_size_kb(kConfigSizesKb[i]);
+ producer_config->set_page_size_kb(kConfigPageSizesKb[i]);
+ }
+
+ consumer->EnableTracing(trace_config);
+ size_t actual_shm_sizes_kb[kNumProducers]{};
+ size_t actual_page_sizes_kb[kNumProducers]{};
+ for (size_t i = 0; i < kNumProducers; i++) {
+ producer[i]->WaitForTracingSetup();
+ producer[i]->WaitForDataSourceStart("data_source");
+ actual_shm_sizes_kb[i] =
+ producer[i]->endpoint()->shared_memory()->size() / 1024;
+ actual_page_sizes_kb[i] =
+ producer[i]->endpoint()->shared_buffer_page_size_kb();
+ }
+ ASSERT_THAT(actual_page_sizes_kb, ElementsAreArray(kExpectedPageSizesKb));
+ ASSERT_THAT(actual_shm_sizes_kb, ElementsAreArray(kExpectedSizesKb));
+}
+
} // namespace perfetto