NotifySharedMemoryUpdate -> CommitData
This CL replaces the NotifySharedMemoryUpdate with a new
CommitData method. The new fields in the CommitDataRequest are
not used yet and will be used in the upcoming CLs.
This CL doesn't introduce any behavioral change.
The only other change here is about getting rid of
the callback in SharedMemoryArbiter. There seems to
be no good reason for that, and just makes the code
harder to read.
Test: perfetto_unittest
Bug: 73612642
Change-Id: I12c9fd7133d1916e30eb31fc50014d64b1e35be0
diff --git a/Android.bp b/Android.bp
index 8879d38..4f5c8de 100644
--- a/Android.bp
+++ b/Android.bp
@@ -57,6 +57,7 @@
"src/traced/probes/probes.cc",
"src/traced/probes/probes_producer.cc",
"src/traced/service/service.cc",
+ "src/tracing/core/commit_data_request.cc",
"src/tracing/core/data_source_config.cc",
"src/tracing/core/data_source_descriptor.cc",
"src/tracing/core/ftrace_config.cc",
@@ -133,6 +134,7 @@
"src/protozero/message_handle.cc",
"src/protozero/proto_utils.cc",
"src/protozero/scattered_stream_writer.cc",
+ "src/tracing/core/commit_data_request.cc",
"src/tracing/core/data_source_config.cc",
"src/tracing/core/data_source_descriptor.cc",
"src/tracing/core/ftrace_config.cc",
@@ -252,6 +254,7 @@
"src/protozero/proto_utils.cc",
"src/protozero/scattered_stream_writer.cc",
"src/traced/probes/probes_producer.cc",
+ "src/tracing/core/commit_data_request.cc",
"src/tracing/core/data_source_config.cc",
"src/tracing/core/data_source_descriptor.cc",
"src/tracing/core/ftrace_config.cc",
@@ -357,6 +360,7 @@
genrule {
name: "perfetto_protos_perfetto_ipc_ipc_gen",
srcs: [
+ "protos/perfetto/ipc/commit_data_request.proto",
"protos/perfetto/ipc/consumer_port.proto",
"protos/perfetto/ipc/producer_port.proto",
],
@@ -366,6 +370,8 @@
],
cmd: "mkdir -p $(genDir)/external/perfetto/protos && $(location aprotoc) --cpp_out=$(genDir)/external/perfetto/protos --proto_path=external/perfetto/protos --plugin=protoc-gen-plugin=$(location perfetto_src_ipc_protoc_plugin_ipc_plugin___gn_standalone_toolchain_gcc_like_host_) --plugin_out=:$(genDir)/external/perfetto/protos $(in)",
out: [
+ "external/perfetto/protos/perfetto/ipc/commit_data_request.ipc.cc",
+ "external/perfetto/protos/perfetto/ipc/commit_data_request.pb.cc",
"external/perfetto/protos/perfetto/ipc/consumer_port.ipc.cc",
"external/perfetto/protos/perfetto/ipc/consumer_port.pb.cc",
"external/perfetto/protos/perfetto/ipc/producer_port.ipc.cc",
@@ -377,6 +383,7 @@
genrule {
name: "perfetto_protos_perfetto_ipc_ipc_gen_headers",
srcs: [
+ "protos/perfetto/ipc/commit_data_request.proto",
"protos/perfetto/ipc/consumer_port.proto",
"protos/perfetto/ipc/producer_port.proto",
],
@@ -386,6 +393,8 @@
],
cmd: "mkdir -p $(genDir)/external/perfetto/protos && $(location aprotoc) --cpp_out=$(genDir)/external/perfetto/protos --proto_path=external/perfetto/protos --plugin=protoc-gen-plugin=$(location perfetto_src_ipc_protoc_plugin_ipc_plugin___gn_standalone_toolchain_gcc_like_host_) --plugin_out=:$(genDir)/external/perfetto/protos $(in)",
out: [
+ "external/perfetto/protos/perfetto/ipc/commit_data_request.ipc.h",
+ "external/perfetto/protos/perfetto/ipc/commit_data_request.pb.h",
"external/perfetto/protos/perfetto/ipc/consumer_port.ipc.h",
"external/perfetto/protos/perfetto/ipc/consumer_port.pb.h",
"external/perfetto/protos/perfetto/ipc/producer_port.ipc.h",
@@ -2849,6 +2858,7 @@
"src/protozero/message_handle.cc",
"src/protozero/proto_utils.cc",
"src/protozero/scattered_stream_writer.cc",
+ "src/tracing/core/commit_data_request.cc",
"src/tracing/core/data_source_config.cc",
"src/tracing/core/data_source_descriptor.cc",
"src/tracing/core/ftrace_config.cc",
@@ -2990,6 +3000,7 @@
"src/protozero/scattered_stream_writer_unittest.cc",
"src/protozero/test/fake_scattered_buffer.cc",
"src/protozero/test/protozero_conformance_unittest.cc",
+ "src/tracing/core/commit_data_request.cc",
"src/tracing/core/data_source_config.cc",
"src/tracing/core/data_source_descriptor.cc",
"src/tracing/core/ftrace_config.cc",
diff --git a/include/perfetto/tracing/core/BUILD.gn b/include/perfetto/tracing/core/BUILD.gn
index 7e39163..1a962b0 100644
--- a/include/perfetto/tracing/core/BUILD.gn
+++ b/include/perfetto/tracing/core/BUILD.gn
@@ -18,6 +18,7 @@
]
sources = [
"basic_types.h",
+ "commit_data_request.h",
"consumer.h",
"data_source_config.h",
"data_source_descriptor.h",
diff --git a/include/perfetto/tracing/core/commit_data_request.h b/include/perfetto/tracing/core/commit_data_request.h
new file mode 100644
index 0000000..6a08d40
--- /dev/null
+++ b/include/perfetto/tracing/core/commit_data_request.h
@@ -0,0 +1,195 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/*******************************************************************************
+ * AUTOGENERATED - DO NOT EDIT
+ *******************************************************************************
+ * This file has been generated from the protobuf message
+ * perfetto/ipc/commit_data_request.proto
+ * by
+ * ../../tools/proto_to_cpp/proto_to_cpp.cc.
+ * If you need to make changes here, change the .proto file and then run
+ * ./tools/gen_tracing_cpp_headers_from_protos.py
+ */
+
+#ifndef INCLUDE_PERFETTO_TRACING_CORE_COMMIT_DATA_REQUEST_H_
+#define INCLUDE_PERFETTO_TRACING_CORE_COMMIT_DATA_REQUEST_H_
+
+#include <stdint.h>
+#include <string>
+#include <type_traits>
+#include <vector>
+
+// Forward declarations for protobuf types.
+namespace perfetto {
+namespace protos {
+class CommitDataRequest;
+class CommitDataRequest_ChunksToMove;
+class CommitDataRequest_ChunkToPatch;
+class CommitDataRequest_ChunkToPatch_Patch;
+} // namespace protos
+} // namespace perfetto
+
+namespace perfetto {
+
+class CommitDataRequest {
+ public:
+ class ChunksToMove {
+ public:
+ ChunksToMove();
+ ~ChunksToMove();
+ ChunksToMove(ChunksToMove&&) noexcept;
+ ChunksToMove& operator=(ChunksToMove&&);
+ ChunksToMove(const ChunksToMove&);
+ ChunksToMove& operator=(const ChunksToMove&);
+
+ // Conversion methods from/to the corresponding protobuf types.
+ void FromProto(const perfetto::protos::CommitDataRequest_ChunksToMove&);
+ void ToProto(perfetto::protos::CommitDataRequest_ChunksToMove*) const;
+
+ uint32_t page() const { return page_; }
+ void set_page(uint32_t value) { page_ = value; }
+
+ uint32_t chunk() const { return chunk_; }
+ void set_chunk(uint32_t value) { chunk_ = value; }
+
+ uint32_t target_buffer() const { return target_buffer_; }
+ void set_target_buffer(uint32_t value) { target_buffer_ = value; }
+
+ private:
+ uint32_t page_ = {};
+ uint32_t chunk_ = {};
+ uint32_t target_buffer_ = {};
+
+ // Allows to preserve unknown protobuf fields for compatibility
+ // with future versions of .proto files.
+ std::string unknown_fields_;
+ };
+
+ class ChunkToPatch {
+ public:
+ class Patch {
+ public:
+ Patch();
+ ~Patch();
+ Patch(Patch&&) noexcept;
+ Patch& operator=(Patch&&);
+ Patch(const Patch&);
+ Patch& operator=(const Patch&);
+
+ // Conversion methods from/to the corresponding protobuf types.
+ void FromProto(
+ const perfetto::protos::CommitDataRequest_ChunkToPatch_Patch&);
+ void ToProto(
+ perfetto::protos::CommitDataRequest_ChunkToPatch_Patch*) const;
+
+ uint32_t offset() const { return offset_; }
+ void set_offset(uint32_t value) { offset_ = value; }
+
+ const std::string& data() const { return data_; }
+ void set_data(const std::string& value) { data_ = value; }
+
+ private:
+ uint32_t offset_ = {};
+ std::string data_ = {};
+
+ // Allows to preserve unknown protobuf fields for compatibility
+ // with future versions of .proto files.
+ std::string unknown_fields_;
+ };
+
+ ChunkToPatch();
+ ~ChunkToPatch();
+ ChunkToPatch(ChunkToPatch&&) noexcept;
+ ChunkToPatch& operator=(ChunkToPatch&&);
+ ChunkToPatch(const ChunkToPatch&);
+ ChunkToPatch& operator=(const ChunkToPatch&);
+
+ // Conversion methods from/to the corresponding protobuf types.
+ void FromProto(const perfetto::protos::CommitDataRequest_ChunkToPatch&);
+ void ToProto(perfetto::protos::CommitDataRequest_ChunkToPatch*) const;
+
+ uint32_t writer_id() const { return writer_id_; }
+ void set_writer_id(uint32_t value) { writer_id_ = value; }
+
+ uint32_t chunk_id() const { return chunk_id_; }
+ void set_chunk_id(uint32_t value) { chunk_id_ = value; }
+
+ int patches_size() const { return static_cast<int>(patches_.size()); }
+ const std::vector<Patch>& patches() const { return patches_; }
+ Patch* add_patches() {
+ patches_.emplace_back();
+ return &patches_.back();
+ }
+
+ bool has_more_patches() const { return has_more_patches_; }
+ void set_has_more_patches(bool value) { has_more_patches_ = value; }
+
+ private:
+ uint32_t writer_id_ = {};
+ uint32_t chunk_id_ = {};
+ std::vector<Patch> patches_;
+ bool has_more_patches_ = {};
+
+ // Allows to preserve unknown protobuf fields for compatibility
+ // with future versions of .proto files.
+ std::string unknown_fields_;
+ };
+
+ CommitDataRequest();
+ ~CommitDataRequest();
+ CommitDataRequest(CommitDataRequest&&) noexcept;
+ CommitDataRequest& operator=(CommitDataRequest&&);
+ CommitDataRequest(const CommitDataRequest&);
+ CommitDataRequest& operator=(const CommitDataRequest&);
+
+ // Conversion methods from/to the corresponding protobuf types.
+ void FromProto(const perfetto::protos::CommitDataRequest&);
+ void ToProto(perfetto::protos::CommitDataRequest*) const;
+
+ int chunks_to_move_size() const {
+ return static_cast<int>(chunks_to_move_.size());
+ }
+ const std::vector<ChunksToMove>& chunks_to_move() const {
+ return chunks_to_move_;
+ }
+ ChunksToMove* add_chunks_to_move() {
+ chunks_to_move_.emplace_back();
+ return &chunks_to_move_.back();
+ }
+
+ int chunks_to_patch_size() const {
+ return static_cast<int>(chunks_to_patch_.size());
+ }
+ const std::vector<ChunkToPatch>& chunks_to_patch() const {
+ return chunks_to_patch_;
+ }
+ ChunkToPatch* add_chunks_to_patch() {
+ chunks_to_patch_.emplace_back();
+ return &chunks_to_patch_.back();
+ }
+
+ private:
+ std::vector<ChunksToMove> chunks_to_move_;
+ std::vector<ChunkToPatch> chunks_to_patch_;
+
+ // Allows to preserve unknown protobuf fields for compatibility
+ // with future versions of .proto files.
+ std::string unknown_fields_;
+};
+
+} // namespace perfetto
+#endif // INCLUDE_PERFETTO_TRACING_CORE_COMMIT_DATA_REQUEST_H_
diff --git a/include/perfetto/tracing/core/service.h b/include/perfetto/tracing/core/service.h
index 5ecdb6a..fcd5c74 100644
--- a/include/perfetto/tracing/core/service.h
+++ b/include/perfetto/tracing/core/service.h
@@ -32,6 +32,7 @@
class TaskRunner;
} // namespace base
+class CommitDataRequest;
class Consumer;
class DataSourceDescriptor;
class Producer;
@@ -74,8 +75,7 @@
// Called by the Producer to signal that some pages in the shared memory
// buffer (shared between Service and Producer) have changed.
- virtual void NotifySharedMemoryUpdate(
- const std::vector<uint32_t>& changed_pages) = 0;
+ virtual void CommitData(const CommitDataRequest&) = 0;
// TODO(primiano): remove this, we shouldn't be exposing the raw
// SHM object but only the TraceWriter (below).
diff --git a/include/perfetto/tracing/core/shared_memory_arbiter.h b/include/perfetto/tracing/core/shared_memory_arbiter.h
index 3bcc12d..e2c3318 100644
--- a/include/perfetto/tracing/core/shared_memory_arbiter.h
+++ b/include/perfetto/tracing/core/shared_memory_arbiter.h
@@ -24,6 +24,7 @@
#include <vector>
#include "perfetto/tracing/core/basic_types.h"
+#include "perfetto/tracing/core/service.h"
namespace perfetto {
@@ -31,6 +32,7 @@
class TaskRunner;
}
+class CommitDataRequest;
class SharedMemory;
class TraceWriter;
@@ -38,9 +40,6 @@
// from the SharedMemory it receives from the Service-side.
class SharedMemoryArbiter {
public:
- using OnPagesCompleteCallback =
- std::function<void(const std::vector<uint32_t>& /*page_indexes*/)>;
-
virtual ~SharedMemoryArbiter() = default;
// Creates a new TraceWriter and assigns it a new WriterID. The WriterID is
@@ -55,7 +54,7 @@
static std::unique_ptr<SharedMemoryArbiter> CreateInstance(
SharedMemory*,
size_t page_size,
- OnPagesCompleteCallback,
+ Service::ProducerEndpoint*,
base::TaskRunner*);
};
diff --git a/protos/perfetto/ipc/BUILD.gn b/protos/perfetto/ipc/BUILD.gn
index 121cfd5..e65cace 100644
--- a/protos/perfetto/ipc/BUILD.gn
+++ b/protos/perfetto/ipc/BUILD.gn
@@ -23,6 +23,7 @@
proto_in_dir = "$perfetto_root_path/protos"
proto_out_dir = "$perfetto_root_path/protos"
sources = [
+ "commit_data_request.proto",
"consumer_port.proto",
"producer_port.proto",
]
diff --git a/protos/perfetto/ipc/commit_data_request.proto b/protos/perfetto/ipc/commit_data_request.proto
new file mode 100644
index 0000000..fec7910
--- /dev/null
+++ b/protos/perfetto/ipc/commit_data_request.proto
@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+syntax = "proto2";
+option optimize_for = LITE_RUNTIME;
+
+package perfetto.protos;
+
+message CommitDataRequest {
+ // When |chunks_to_move| is present, the producer is requesting the service to
+ // move the given chunks form the share memory buffer into the central
+ // trace buffer(s).
+ message ChunksToMove {
+ // The 0-based index of the page in the Shared Memory Buffer.
+ optional uint32 page = 1;
+
+ // The 0-based chunk index [0..13] within the page.
+ optional uint32 chunk = 2;
+
+ // The target buffer it should be moved onto. The service will check that
+ // the producer is allowed to write into that buffer before the move.
+ optional uint32 target_buffer = 3;
+ }
+ repeated ChunksToMove chunks_to_move = 1;
+
+ // Used to patch chunks that have already been sent to the service. The chunk
+ // might not be in the shared memory buffer anymore as it could have been
+ // moved by the service in response to a prior CommitDataRequest.
+ // It is perfectly valid to patch a chunk that is being notified in the same
+ // message (a chunk can show up both in the |changed_pages| and |patches|
+ // field within the same CommitDataRequest message).
+ // In other words, |chunks_to_patch| is always processed after
+ // |chunks_to_move|.
+ message ChunkToPatch {
+ message Patch {
+ // Offset in bytes from the start of the chunk payload. e.g., offset == 0
+ // corresponds to the first byte of the first packet (or fragment) in the
+ // chunk.
+ optional uint32 offset = 1;
+
+ // Bytes to patch at the given offset.
+ optional bytes data = 2;
+ }
+ // {WriterID, ChunkID} uniquely identify a chunk for the current producer.
+ optional uint32 writer_id = 1;
+ optional uint32 chunk_id = 2;
+
+ // List of patches to apply to the given chunk.
+ repeated Patch patches = 3;
+
+ // When true more patches will follow in future requests and the chunk
+ // should be still considered as patch-pending. When false the chunk becomes
+ // eligible for reading.
+ optional bool has_more_patches = 4;
+ }
+ repeated ChunkToPatch chunks_to_patch = 2;
+}
diff --git a/protos/perfetto/ipc/producer_port.proto b/protos/perfetto/ipc/producer_port.proto
index dda5991..572e83d 100644
--- a/protos/perfetto/ipc/producer_port.proto
+++ b/protos/perfetto/ipc/producer_port.proto
@@ -19,6 +19,7 @@
import "perfetto/config/data_source_config.proto";
import "perfetto/config/data_source_descriptor.proto";
+import "perfetto/ipc/commit_data_request.proto";
package perfetto.protos;
@@ -37,12 +38,10 @@
rpc UnregisterDataSource(UnregisterDataSourceRequest)
returns (UnregisterDataSourceResponse) {}
- // Sent by the client whenever it the state of one or more pages in the shared
- // memory buffer have been changed. This is used to kick the tracing service
- // and let it move filled pages from the staging shared memory buffer into the
- // actual (non-shared) trace buffer.
- rpc NotifySharedMemoryUpdate(NotifySharedMemoryUpdateRequest)
- returns (NotifySharedMemoryUpdateResponse) {}
+ // Sent by the client to request the service to:
+ // 1) Move some chunks from the shmem buffer into the logging buffer.
+ // 2) Patch the content of some chunks previously moved.
+ rpc CommitData(protos.CommitDataRequest) returns (CommitDataResponse) {}
// This is a backchannel to get asynchronous commands / notifications back
// from the Service.
@@ -90,13 +89,12 @@
message UnregisterDataSourceResponse {}
-// Arguments for rpc NotifySharedMemoryUpdate().
+// Arguments for rpc CommitData().
+// See commit_data_request.proto for CommitDataRequest. That has its own file
+// because it is used also as input to generate the C++ classes in tracing/core
+// via tools/gen_tracing_cpp_headers_from_protos.py.
-message NotifySharedMemoryUpdateRequest {
- repeated uint32 changed_pages = 1;
-}
-
-message NotifySharedMemoryUpdateResponse {}
+message CommitDataResponse {}
// Arguments for rpc GetAsyncCommand().
diff --git a/src/tracing/BUILD.gn b/src/tracing/BUILD.gn
index 567be03..3a24f6a 100644
--- a/src/tracing/BUILD.gn
+++ b/src/tracing/BUILD.gn
@@ -29,6 +29,7 @@
"../base",
]
sources = [
+ "core/commit_data_request.cc",
"core/data_source_config.cc",
"core/data_source_descriptor.cc",
"core/ftrace_config.cc",
diff --git a/src/tracing/core/commit_data_request.cc b/src/tracing/core/commit_data_request.cc
new file mode 100644
index 0000000..ad40917
--- /dev/null
+++ b/src/tracing/core/commit_data_request.cc
@@ -0,0 +1,206 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/*******************************************************************************
+ * AUTOGENERATED - DO NOT EDIT
+ *******************************************************************************
+ * This file has been generated from the protobuf message
+ * perfetto/ipc/commit_data_request.proto
+ * by
+ * ../../tools/proto_to_cpp/proto_to_cpp.cc.
+ * If you need to make changes here, change the .proto file and then run
+ * ./tools/gen_tracing_cpp_headers_from_protos.py
+ */
+
+#include "perfetto/tracing/core/commit_data_request.h"
+
+#include "perfetto/ipc/commit_data_request.pb.h"
+
+namespace perfetto {
+
+CommitDataRequest::CommitDataRequest() = default;
+CommitDataRequest::~CommitDataRequest() = default;
+CommitDataRequest::CommitDataRequest(const CommitDataRequest&) = default;
+CommitDataRequest& CommitDataRequest::operator=(const CommitDataRequest&) =
+ default;
+CommitDataRequest::CommitDataRequest(CommitDataRequest&&) noexcept = default;
+CommitDataRequest& CommitDataRequest::operator=(CommitDataRequest&&) = default;
+
+void CommitDataRequest::FromProto(
+ const perfetto::protos::CommitDataRequest& proto) {
+ chunks_to_move_.clear();
+ for (const auto& field : proto.chunks_to_move()) {
+ chunks_to_move_.emplace_back();
+ chunks_to_move_.back().FromProto(field);
+ }
+
+ chunks_to_patch_.clear();
+ for (const auto& field : proto.chunks_to_patch()) {
+ chunks_to_patch_.emplace_back();
+ chunks_to_patch_.back().FromProto(field);
+ }
+ unknown_fields_ = proto.unknown_fields();
+}
+
+void CommitDataRequest::ToProto(
+ perfetto::protos::CommitDataRequest* proto) const {
+ proto->Clear();
+
+ for (const auto& it : chunks_to_move_) {
+ auto* entry = proto->add_chunks_to_move();
+ it.ToProto(entry);
+ }
+
+ for (const auto& it : chunks_to_patch_) {
+ auto* entry = proto->add_chunks_to_patch();
+ it.ToProto(entry);
+ }
+ *(proto->mutable_unknown_fields()) = unknown_fields_;
+}
+
+CommitDataRequest::ChunksToMove::ChunksToMove() = default;
+CommitDataRequest::ChunksToMove::~ChunksToMove() = default;
+CommitDataRequest::ChunksToMove::ChunksToMove(
+ const CommitDataRequest::ChunksToMove&) = default;
+CommitDataRequest::ChunksToMove& CommitDataRequest::ChunksToMove::operator=(
+ const CommitDataRequest::ChunksToMove&) = default;
+CommitDataRequest::ChunksToMove::ChunksToMove(
+ CommitDataRequest::ChunksToMove&&) noexcept = default;
+CommitDataRequest::ChunksToMove& CommitDataRequest::ChunksToMove::operator=(
+ CommitDataRequest::ChunksToMove&&) = default;
+
+void CommitDataRequest::ChunksToMove::FromProto(
+ const perfetto::protos::CommitDataRequest_ChunksToMove& proto) {
+ static_assert(sizeof(page_) == sizeof(proto.page()), "size mismatch");
+ page_ = static_cast<decltype(page_)>(proto.page());
+
+ static_assert(sizeof(chunk_) == sizeof(proto.chunk()), "size mismatch");
+ chunk_ = static_cast<decltype(chunk_)>(proto.chunk());
+
+ static_assert(sizeof(target_buffer_) == sizeof(proto.target_buffer()),
+ "size mismatch");
+ target_buffer_ = static_cast<decltype(target_buffer_)>(proto.target_buffer());
+ unknown_fields_ = proto.unknown_fields();
+}
+
+void CommitDataRequest::ChunksToMove::ToProto(
+ perfetto::protos::CommitDataRequest_ChunksToMove* proto) const {
+ proto->Clear();
+
+ static_assert(sizeof(page_) == sizeof(proto->page()), "size mismatch");
+ proto->set_page(static_cast<decltype(proto->page())>(page_));
+
+ static_assert(sizeof(chunk_) == sizeof(proto->chunk()), "size mismatch");
+ proto->set_chunk(static_cast<decltype(proto->chunk())>(chunk_));
+
+ static_assert(sizeof(target_buffer_) == sizeof(proto->target_buffer()),
+ "size mismatch");
+ proto->set_target_buffer(
+ static_cast<decltype(proto->target_buffer())>(target_buffer_));
+ *(proto->mutable_unknown_fields()) = unknown_fields_;
+}
+
+CommitDataRequest::ChunkToPatch::ChunkToPatch() = default;
+CommitDataRequest::ChunkToPatch::~ChunkToPatch() = default;
+CommitDataRequest::ChunkToPatch::ChunkToPatch(
+ const CommitDataRequest::ChunkToPatch&) = default;
+CommitDataRequest::ChunkToPatch& CommitDataRequest::ChunkToPatch::operator=(
+ const CommitDataRequest::ChunkToPatch&) = default;
+CommitDataRequest::ChunkToPatch::ChunkToPatch(
+ CommitDataRequest::ChunkToPatch&&) noexcept = default;
+CommitDataRequest::ChunkToPatch& CommitDataRequest::ChunkToPatch::operator=(
+ CommitDataRequest::ChunkToPatch&&) = default;
+
+void CommitDataRequest::ChunkToPatch::FromProto(
+ const perfetto::protos::CommitDataRequest_ChunkToPatch& proto) {
+ static_assert(sizeof(writer_id_) == sizeof(proto.writer_id()),
+ "size mismatch");
+ writer_id_ = static_cast<decltype(writer_id_)>(proto.writer_id());
+
+ static_assert(sizeof(chunk_id_) == sizeof(proto.chunk_id()), "size mismatch");
+ chunk_id_ = static_cast<decltype(chunk_id_)>(proto.chunk_id());
+
+ patches_.clear();
+ for (const auto& field : proto.patches()) {
+ patches_.emplace_back();
+ patches_.back().FromProto(field);
+ }
+
+ static_assert(sizeof(has_more_patches_) == sizeof(proto.has_more_patches()),
+ "size mismatch");
+ has_more_patches_ =
+ static_cast<decltype(has_more_patches_)>(proto.has_more_patches());
+ unknown_fields_ = proto.unknown_fields();
+}
+
+void CommitDataRequest::ChunkToPatch::ToProto(
+ perfetto::protos::CommitDataRequest_ChunkToPatch* proto) const {
+ proto->Clear();
+
+ static_assert(sizeof(writer_id_) == sizeof(proto->writer_id()),
+ "size mismatch");
+ proto->set_writer_id(static_cast<decltype(proto->writer_id())>(writer_id_));
+
+ static_assert(sizeof(chunk_id_) == sizeof(proto->chunk_id()),
+ "size mismatch");
+ proto->set_chunk_id(static_cast<decltype(proto->chunk_id())>(chunk_id_));
+
+ for (const auto& it : patches_) {
+ auto* entry = proto->add_patches();
+ it.ToProto(entry);
+ }
+
+ static_assert(sizeof(has_more_patches_) == sizeof(proto->has_more_patches()),
+ "size mismatch");
+ proto->set_has_more_patches(
+ static_cast<decltype(proto->has_more_patches())>(has_more_patches_));
+ *(proto->mutable_unknown_fields()) = unknown_fields_;
+}
+
+CommitDataRequest::ChunkToPatch::Patch::Patch() = default;
+CommitDataRequest::ChunkToPatch::Patch::~Patch() = default;
+CommitDataRequest::ChunkToPatch::Patch::Patch(
+ const CommitDataRequest::ChunkToPatch::Patch&) = default;
+CommitDataRequest::ChunkToPatch::Patch& CommitDataRequest::ChunkToPatch::Patch::
+operator=(const CommitDataRequest::ChunkToPatch::Patch&) = default;
+CommitDataRequest::ChunkToPatch::Patch::Patch(
+ CommitDataRequest::ChunkToPatch::Patch&&) noexcept = default;
+CommitDataRequest::ChunkToPatch::Patch& CommitDataRequest::ChunkToPatch::Patch::
+operator=(CommitDataRequest::ChunkToPatch::Patch&&) = default;
+
+void CommitDataRequest::ChunkToPatch::Patch::FromProto(
+ const perfetto::protos::CommitDataRequest_ChunkToPatch_Patch& proto) {
+ static_assert(sizeof(offset_) == sizeof(proto.offset()), "size mismatch");
+ offset_ = static_cast<decltype(offset_)>(proto.offset());
+
+ static_assert(sizeof(data_) == sizeof(proto.data()), "size mismatch");
+ data_ = static_cast<decltype(data_)>(proto.data());
+ unknown_fields_ = proto.unknown_fields();
+}
+
+void CommitDataRequest::ChunkToPatch::Patch::ToProto(
+ perfetto::protos::CommitDataRequest_ChunkToPatch_Patch* proto) const {
+ proto->Clear();
+
+ static_assert(sizeof(offset_) == sizeof(proto->offset()), "size mismatch");
+ proto->set_offset(static_cast<decltype(proto->offset())>(offset_));
+
+ static_assert(sizeof(data_) == sizeof(proto->data()), "size mismatch");
+ proto->set_data(static_cast<decltype(proto->data())>(data_));
+ *(proto->mutable_unknown_fields()) = unknown_fields_;
+}
+
+} // namespace perfetto
diff --git a/src/tracing/core/service_impl.cc b/src/tracing/core/service_impl.cc
index e4a8f51..5f30a87 100644
--- a/src/tracing/core/service_impl.cc
+++ b/src/tracing/core/service_impl.cc
@@ -25,6 +25,7 @@
#include "perfetto/base/task_runner.h"
#include "perfetto/base/utils.h"
#include "perfetto/protozero/proto_utils.h"
+#include "perfetto/tracing/core/commit_data_request.h"
#include "perfetto/tracing/core/consumer.h"
#include "perfetto/tracing/core/data_source_config.h"
#include "perfetto/tracing/core/producer.h"
@@ -697,21 +698,28 @@
service_->UnregisterDataSource(id_, ds_id);
}
-void ServiceImpl::ProducerEndpointImpl::NotifySharedMemoryUpdate(
- const std::vector<uint32_t>& changed_pages) {
+void ServiceImpl::ProducerEndpointImpl::CommitData(
+ const CommitDataRequest& req_untrusted) {
PERFETTO_DCHECK_THREAD(thread_checker_);
- for (uint32_t page_idx : changed_pages) {
+
+ for (const auto& chunks : req_untrusted.chunks_to_move()) {
+ const uint32_t page_idx = chunks.page();
if (page_idx >= shmem_abi_.num_pages())
- continue; // Very likely a malicious producer playing dirty.
+ continue; // A buggy or malicious producer.
if (!shmem_abi_.is_page_complete(page_idx))
continue;
+
+ // TODO(primiano): implement per-chunk move.
+ PERFETTO_DCHECK(chunks.chunk() == 0);
+
if (!shmem_abi_.TryAcquireAllChunksForReading(page_idx))
continue;
// TODO(fmayer): we should start collecting individual chunks from non fully
// complete pages after a while.
+ // TODO(primiano): in next CL, use chunks.target_buffer() instead.
service_->CopyProducerPageIntoLogBuffer(
id_, shmem_abi_.get_target_buffer(page_idx),
shmem_abi_.page_start(page_idx), shmem_abi_.page_size());
diff --git a/src/tracing/core/service_impl.h b/src/tracing/core/service_impl.h
index 60b8587..7725c12 100644
--- a/src/tracing/core/service_impl.h
+++ b/src/tracing/core/service_impl.h
@@ -64,8 +64,7 @@
void RegisterDataSource(const DataSourceDescriptor&,
RegisterDataSourceCallback) override;
void UnregisterDataSource(DataSourceID) override;
- void NotifySharedMemoryUpdate(
- const std::vector<uint32_t>& changed_pages) override;
+ void CommitData(const CommitDataRequest&) override;
std::unique_ptr<TraceWriter> CreateTraceWriter(BufferID) override;
SharedMemory* shared_memory() const override;
diff --git a/src/tracing/core/shared_memory_arbiter_impl.cc b/src/tracing/core/shared_memory_arbiter_impl.cc
index 528f2c2..2e1bc34 100644
--- a/src/tracing/core/shared_memory_arbiter_impl.cc
+++ b/src/tracing/core/shared_memory_arbiter_impl.cc
@@ -18,6 +18,7 @@
#include "perfetto/base/logging.h"
#include "perfetto/base/task_runner.h"
+#include "perfetto/tracing/core/commit_data_request.h"
#include "perfetto/tracing/core/shared_memory.h"
#include "src/tracing/core/trace_writer_impl.h"
@@ -36,22 +37,24 @@
std::unique_ptr<SharedMemoryArbiter> SharedMemoryArbiter::CreateInstance(
SharedMemory* shared_memory,
size_t page_size,
- OnPagesCompleteCallback callback,
+ Service::ProducerEndpoint* producer_endpoint,
base::TaskRunner* task_runner) {
return std::unique_ptr<SharedMemoryArbiterImpl>(
new SharedMemoryArbiterImpl(shared_memory->start(), shared_memory->size(),
- page_size, std::move(callback), task_runner));
+ page_size, producer_endpoint, task_runner));
}
+
SharedMemoryArbiterImpl::SharedMemoryArbiterImpl(
void* start,
size_t size,
size_t page_size,
- OnPagesCompleteCallback callback,
+ Service::ProducerEndpoint* producer_endpoint,
base::TaskRunner* task_runner)
: task_runner_(task_runner),
- on_pages_complete_callback_(std::move(callback)),
+ producer_endpoint_(producer_endpoint),
shmem_abi_(reinterpret_cast<uint8_t*>(start), size, page_size),
- active_writer_ids_(kMaxWriterID) {}
+ active_writer_ids_(kMaxWriterID),
+ weak_ptr_factory_(this) {}
Chunk SharedMemoryArbiterImpl::GetNewChunk(
const SharedMemoryABI::ChunkHeader& header,
@@ -142,7 +145,10 @@
// tracing service to consume the shared memory buffer (SMB) and, for
// this reason, never run the task that tells the service to purge the
// SMB.
- NotifySharedMemoryUpdate();
+ // TODO(primiano): We cannot call this if we aren't on the |task_runner_|
+ // thread. Works for now because all traced_probes writes happen on the
+ // main thread.
+ SendPendingCommitDataRequest();
}
usleep(kStallIntervalUs);
}
@@ -150,32 +156,48 @@
void SharedMemoryArbiterImpl::ReturnCompletedChunk(Chunk chunk) {
bool should_post_callback = false;
+ base::WeakPtr<SharedMemoryArbiterImpl> weak_this;
{
std::lock_guard<std::mutex> scoped_lock(lock_);
size_t page_index = shmem_abi_.ReleaseChunkAsComplete(std::move(chunk));
+
if (page_index != SharedMemoryABI::kInvalidPageIdx) {
- should_post_callback = pages_to_notify_.empty();
- pages_to_notify_.push_back(static_cast<uint32_t>(page_index));
+ if (!commit_data_req_) {
+ commit_data_req_.reset(new CommitDataRequest());
+ weak_this = weak_ptr_factory_.GetWeakPtr();
+ should_post_callback = true;
+ }
+ CommitDataRequest::ChunksToMove* ctm =
+ commit_data_req_->add_chunks_to_move();
+ ctm->set_page(static_cast<uint32_t>(page_index));
+ // TODO(primiano): implement per-chunk notifications.
+ // ctm->add_chunk_numbers(...)
+ // ctm->set_target_buffer(target_buffer);
}
}
if (should_post_callback) {
- // TODO(primiano): what happens if the arbiter gets destroyed?
- task_runner_->PostTask(
- std::bind(&SharedMemoryArbiterImpl::NotifySharedMemoryUpdate, this));
+ PERFETTO_DCHECK(weak_this);
+ task_runner_->PostTask([weak_this] {
+ if (weak_this)
+ weak_this->SendPendingCommitDataRequest();
+ });
}
}
// This is always invoked on the |task_runner_| thread.
-void SharedMemoryArbiterImpl::NotifySharedMemoryUpdate() {
+void SharedMemoryArbiterImpl::SendPendingCommitDataRequest() {
+ PERFETTO_DCHECK_THREAD(thread_checker_);
+ std::unique_ptr<CommitDataRequest> req;
std::vector<uint32_t> pages_to_notify;
{
std::lock_guard<std::mutex> scoped_lock(lock_);
- pages_to_notify = std::move(pages_to_notify_);
- pages_to_notify_.clear();
+ req = std::move(commit_data_req_);
}
- if (!pages_to_notify.empty())
- on_pages_complete_callback_(pages_to_notify);
+ // |commit_data_req_| could become nullptr if the forced sync flush happens
+ // in GetNewChunk().
+ if (req)
+ producer_endpoint_->CommitData(*req);
}
std::unique_ptr<TraceWriter> SharedMemoryArbiterImpl::CreateTraceWriter(
diff --git a/src/tracing/core/shared_memory_arbiter_impl.h b/src/tracing/core/shared_memory_arbiter_impl.h
index f2bd599..886f4d2 100644
--- a/src/tracing/core/shared_memory_arbiter_impl.h
+++ b/src/tracing/core/shared_memory_arbiter_impl.h
@@ -20,9 +20,12 @@
#include <stdint.h>
#include <functional>
+#include <memory>
#include <mutex>
#include <vector>
+#include "perfetto/base/thread_checker.h"
+#include "perfetto/base/weak_ptr.h"
#include "perfetto/tracing/core/basic_types.h"
#include "perfetto/tracing/core/shared_memory_abi.h"
#include "perfetto/tracing/core/shared_memory_arbiter.h"
@@ -30,6 +33,7 @@
namespace perfetto {
+class CommitDataRequest;
class TraceWriter;
namespace base {
@@ -50,11 +54,11 @@
// pages. See tradeoff considerations in shared_memory_abi.h.
// |OnPagesCompleteCallback|: a callback that will be posted on the passed
// |TaskRunner| when one or more pages are complete (and hence the Producer
- // should send a NotifySharedMemoryUpdate() to the Service).
+ // should send a CommitData request to the Service).
SharedMemoryArbiterImpl(void* start,
size_t size,
size_t page_size,
- OnPagesCompleteCallback,
+ Service::ProducerEndpoint*,
base::TaskRunner*);
// Returns a new Chunk to write tracing data. The call always returns a valid
@@ -89,18 +93,22 @@
// Called by the TraceWriter destructor.
void ReleaseWriterID(WriterID);
- void NotifySharedMemoryUpdate();
+ void SendPendingCommitDataRequest();
base::TaskRunner* const task_runner_;
- OnPagesCompleteCallback on_pages_complete_callback_;
+ Service::ProducerEndpoint* const producer_endpoint_;
+ PERFETTO_THREAD_CHECKER(thread_checker_)
// --- Begin lock-protected members ---
std::mutex lock_;
SharedMemoryABI shmem_abi_;
size_t page_idx_ = 0;
+ std::unique_ptr<CommitDataRequest> commit_data_req_;
IdAllocator<WriterID> active_writer_ids_;
- std::vector<uint32_t> pages_to_notify_;
// --- End lock-protected members ---
+
+ // Keep at the end.
+ base::WeakPtrFactory<SharedMemoryArbiterImpl> weak_ptr_factory_;
};
} // namespace perfetto
diff --git a/src/tracing/core/shared_memory_arbiter_impl_unittest.cc b/src/tracing/core/shared_memory_arbiter_impl_unittest.cc
index 4de44e0..c311787 100644
--- a/src/tracing/core/shared_memory_arbiter_impl_unittest.cc
+++ b/src/tracing/core/shared_memory_arbiter_impl_unittest.cc
@@ -16,9 +16,11 @@
#include "src/tracing/core/shared_memory_arbiter_impl.h"
+#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "perfetto/base/utils.h"
#include "perfetto/tracing/core/basic_types.h"
+#include "perfetto/tracing/core/commit_data_request.h"
#include "perfetto/tracing/core/shared_memory_abi.h"
#include "perfetto/tracing/core/trace_writer.h"
#include "src/base/test/test_task_runner.h"
@@ -27,17 +29,30 @@
namespace perfetto {
namespace {
+using testing::Invoke;
+using testing::_;
+
+class MockProducerEndpoint : public Service::ProducerEndpoint {
+ public:
+ void RegisterDataSource(const DataSourceDescriptor&,
+ RegisterDataSourceCallback) override {}
+ void UnregisterDataSource(DataSourceID) override {}
+ SharedMemory* shared_memory() const override { return nullptr; }
+ std::unique_ptr<TraceWriter> CreateTraceWriter(BufferID) override {
+ return nullptr;
+ }
+
+ MOCK_METHOD1(CommitData, void(const CommitDataRequest&));
+};
+
class SharedMemoryArbiterImplTest : public AlignedBufferTest {
public:
void SetUp() override {
AlignedBufferTest::SetUp();
- auto callback = [this](const std::vector<uint32_t>& arg) {
- if (on_pages_complete_)
- on_pages_complete_(arg);
- };
task_runner_.reset(new base::TestTaskRunner());
arbiter_.reset(new SharedMemoryArbiterImpl(buf(), buf_size(), page_size(),
- callback, task_runner_.get()));
+ &mock_producer_endpoint_,
+ task_runner_.get()));
}
void TearDown() override {
@@ -47,6 +62,7 @@
std::unique_ptr<base::TestTaskRunner> task_runner_;
std::unique_ptr<SharedMemoryArbiterImpl> arbiter_;
+ MockProducerEndpoint mock_producer_endpoint_;
std::function<void(const std::vector<uint32_t>&)> on_pages_complete_;
};
@@ -138,13 +154,14 @@
// check that the notification callback is posted.
auto on_callback = task_runner_->CreateCheckpoint("on_callback");
- on_pages_complete_ =
- [on_callback](const std::vector<uint32_t>& completed_pages) {
- ASSERT_EQ(2u, completed_pages.size());
- ASSERT_EQ(0u, completed_pages[0]);
- ASSERT_EQ(3u, completed_pages[1]);
+ EXPECT_CALL(mock_producer_endpoint_, CommitData(_))
+ .WillOnce(Invoke([on_callback](const CommitDataRequest& req) {
+ ASSERT_EQ(2, req.chunks_to_move_size());
+ ASSERT_EQ(0u, req.chunks_to_move()[0].page());
+ ASSERT_EQ(3u, req.chunks_to_move()[1].page());
+ // TODO(primiano): In next CL, ASSERT_EQ on buffer and chunk number.
on_callback();
- };
+ }));
for (size_t i = 0; i < 14; i++) {
arbiter_->ReturnCompletedChunk(std::move(chunks[14 * i]));
arbiter_->ReturnCompletedChunk(std::move(chunks[14 * i + 3]));
diff --git a/src/tracing/core/trace_buffer.cc b/src/tracing/core/trace_buffer.cc
index 55615a5..3c936ba 100644
--- a/src/tracing/core/trace_buffer.cc
+++ b/src/tracing/core/trace_buffer.cc
@@ -216,7 +216,7 @@
// zeroes from here to end().
// TODO(primiano): optimization: if during Initialize() we fill the buffer
// with padding records we could get rid of this branch.
- if (!next_chunk.is_valid()) {
+ if (PERFETTO_UNLIKELY(!next_chunk.is_valid())) {
// This should happen only at the first iteration. The zeroed area can
// only begin precisely at the |wptr_|, not after. Otherwise it means that
// we wrapped but screwed up the ChunkRecord chain.
diff --git a/src/tracing/core/trace_buffer.h b/src/tracing/core/trace_buffer.h
index 29a70c6..03fcf51 100644
--- a/src/tracing/core/trace_buffer.h
+++ b/src/tracing/core/trace_buffer.h
@@ -35,7 +35,7 @@
// ultimately stored into. The service will own several instances of this class,
// at least one per active consumer (as defined in the |buffers| section of
// trace_config.proto) and will copy chunks from the producer's shared memory
-// buffers into here when a NotifySharedMemoryUpdate IPC is received.
+// buffers into here when a CommitData IPC is received.
//
// Writing into the buffer
// -----------------------
diff --git a/src/tracing/core/trace_writer_impl_unittest.cc b/src/tracing/core/trace_writer_impl_unittest.cc
index 3b65448..b542054 100644
--- a/src/tracing/core/trace_writer_impl_unittest.cc
+++ b/src/tracing/core/trace_writer_impl_unittest.cc
@@ -18,6 +18,8 @@
#include "gtest/gtest.h"
#include "perfetto/base/utils.h"
+#include "perfetto/tracing/core/commit_data_request.h"
+#include "perfetto/tracing/core/service.h"
#include "perfetto/tracing/core/trace_writer.h"
#include "src/base/test/test_task_runner.h"
#include "src/tracing/core/shared_memory_arbiter_impl.h"
@@ -29,16 +31,27 @@
namespace perfetto {
namespace {
+class FakeProducerEndpoint : public Service::ProducerEndpoint {
+ void RegisterDataSource(const DataSourceDescriptor&,
+ RegisterDataSourceCallback) override {}
+ void UnregisterDataSource(DataSourceID) override {}
+ void CommitData(const CommitDataRequest&) override {}
+ SharedMemory* shared_memory() const override { return nullptr; }
+ std::unique_ptr<TraceWriter> CreateTraceWriter(BufferID) override {
+ return nullptr;
+ }
+};
+
class TraceWriterImplTest : public AlignedBufferTest {
public:
void SetUp() override {
SharedMemoryArbiterImpl::set_default_layout_for_testing(
SharedMemoryABI::PageLayout::kPageDiv4);
AlignedBufferTest::SetUp();
- auto callback = [](const std::vector<uint32_t>& arg) {};
task_runner_.reset(new base::TestTaskRunner());
arbiter_.reset(new SharedMemoryArbiterImpl(buf(), buf_size(), page_size(),
- callback, task_runner_.get()));
+ &fake_producer_endpoint_,
+ task_runner_.get()));
}
void TearDown() override {
@@ -46,6 +59,7 @@
task_runner_.reset();
}
+ FakeProducerEndpoint fake_producer_endpoint_;
std::unique_ptr<base::TestTaskRunner> task_runner_;
std::unique_ptr<SharedMemoryArbiterImpl> arbiter_;
std::function<void(const std::vector<uint32_t>&)> on_pages_complete_;
diff --git a/src/tracing/ipc/producer/producer_ipc_client_impl.cc b/src/tracing/ipc/producer/producer_ipc_client_impl.cc
index 2a7b090..94ca426 100644
--- a/src/tracing/ipc/producer/producer_ipc_client_impl.cc
+++ b/src/tracing/ipc/producer/producer_ipc_client_impl.cc
@@ -21,6 +21,7 @@
#include "perfetto/base/task_runner.h"
#include "perfetto/ipc/client.h"
+#include "perfetto/tracing/core/commit_data_request.h"
#include "perfetto/tracing/core/data_source_config.h"
#include "perfetto/tracing/core/data_source_descriptor.h"
#include "perfetto/tracing/core/producer.h"
@@ -103,32 +104,12 @@
// TODO(primiano): handle mmap failure in case of OOM.
shared_memory_ = PosixSharedMemory::AttachToFd(std::move(shmem_fd));
- auto on_pages_complete = [this](const std::vector<uint32_t>& changed_pages) {
- OnPagesComplete(changed_pages);
- };
shared_memory_arbiter_ = SharedMemoryArbiter::CreateInstance(
- shared_memory_.get(), kBufferPageSize, on_pages_complete, task_runner_);
+ shared_memory_.get(), kBufferPageSize, this, task_runner_);
producer_->OnConnect();
}
-// Called by SharedMemoryArbiterImpl when some chunks are complete and we need
-// to notify the service about that.
-void ProducerIPCClientImpl::OnPagesComplete(
- const std::vector<uint32_t>& changed_pages) {
- PERFETTO_DCHECK_THREAD(thread_checker_);
- if (!connected_) {
- PERFETTO_DLOG("Cannot OnPagesComplete(), not connected to tracing service");
- return;
- }
- protos::NotifySharedMemoryUpdateRequest req;
- for (uint32_t page_idx : changed_pages)
- req.add_changed_pages(page_idx);
-
- producer_port_.NotifySharedMemoryUpdate(
- req, ipc::Deferred<protos::NotifySharedMemoryUpdateResponse>());
-}
-
void ProducerIPCClientImpl::OnServiceRequest(
const protos::GetAsyncCommandResponse& cmd) {
PERFETTO_DCHECK_THREAD(thread_checker_);
@@ -197,19 +178,16 @@
req, ipc::Deferred<protos::UnregisterDataSourceResponse>());
}
-void ProducerIPCClientImpl::NotifySharedMemoryUpdate(
- const std::vector<uint32_t>& changed_pages) {
+void ProducerIPCClientImpl::CommitData(const CommitDataRequest& req) {
PERFETTO_DCHECK_THREAD(thread_checker_);
if (!connected_) {
- PERFETTO_DLOG(
- "Cannot NotifySharedMemoryUpdate(), not connected to tracing service");
+ PERFETTO_DLOG("Cannot CommitData(), not connected to tracing service");
return;
}
- protos::NotifySharedMemoryUpdateRequest req;
- for (uint32_t changed_page : changed_pages)
- req.add_changed_pages(changed_page);
- producer_port_.NotifySharedMemoryUpdate(
- req, ipc::Deferred<protos::NotifySharedMemoryUpdateResponse>());
+ protos::CommitDataRequest proto_req;
+ req.ToProto(&proto_req);
+ producer_port_.CommitData(proto_req,
+ ipc::Deferred<protos::CommitDataResponse>());
}
std::unique_ptr<TraceWriter> ProducerIPCClientImpl::CreateTraceWriter(
diff --git a/src/tracing/ipc/producer/producer_ipc_client_impl.h b/src/tracing/ipc/producer/producer_ipc_client_impl.h
index ebd9a51..166c64f 100644
--- a/src/tracing/ipc/producer/producer_ipc_client_impl.h
+++ b/src/tracing/ipc/producer/producer_ipc_client_impl.h
@@ -62,8 +62,7 @@
void RegisterDataSource(const DataSourceDescriptor&,
RegisterDataSourceCallback) override;
void UnregisterDataSource(DataSourceID) override;
- void NotifySharedMemoryUpdate(
- const std::vector<uint32_t>& changed_pages) override;
+ void CommitData(const CommitDataRequest&) override;
std::unique_ptr<TraceWriter> CreateTraceWriter(
BufferID target_buffer) override;
SharedMemory* shared_memory() const override;
@@ -82,9 +81,6 @@
// (e.g. start/stop a data source).
void OnServiceRequest(const protos::GetAsyncCommandResponse&);
- // Callback passed to SharedMemoryArbiterImpl.
- void OnPagesComplete(const std::vector<uint32_t>&);
-
// TODO think to destruction order, do we rely on any specific dtor sequence?
Producer* const producer_;
base::TaskRunner* const task_runner_;
diff --git a/src/tracing/ipc/service/producer_ipc_service.cc b/src/tracing/ipc/service/producer_ipc_service.cc
index 529db01..ea4ea97 100644
--- a/src/tracing/ipc/service/producer_ipc_service.cc
+++ b/src/tracing/ipc/service/producer_ipc_service.cc
@@ -21,6 +21,7 @@
#include "perfetto/base/logging.h"
#include "perfetto/base/task_runner.h"
#include "perfetto/ipc/host.h"
+#include "perfetto/tracing/core/commit_data_request.h"
#include "perfetto/tracing/core/data_source_config.h"
#include "perfetto/tracing/core/data_source_descriptor.h"
#include "perfetto/tracing/core/service.h"
@@ -174,27 +175,20 @@
ipc::AsyncResult<protos::UnregisterDataSourceResponse>::Create());
}
-void ProducerIPCService::NotifySharedMemoryUpdate(
- const protos::NotifySharedMemoryUpdateRequest& req,
- DeferredNotifySharedMemoryUpdateResponse) {
- // The response object is deliberately not resolved. NotifySharedMemoryUpdate
- // messages don't expect any response (i.e. the client sends them with the
- // |drop_reply| flag). This is to avoid useless wakeups on the client side.
+void ProducerIPCService::CommitData(const protos::CommitDataRequest& proto_req,
+ DeferredCommitDataResponse) {
+ // The response object is deliberately not resolved. CommitData messages don't
+ // expect any response (the client sends them with the |drop_reply| flag).
+ // This is to avoid useless wakeups on the client side.
RemoteProducer* producer = GetProducerForCurrentRequest();
if (!producer) {
PERFETTO_DLOG(
- "Producer invoked NotifySharedMemoryUpdate() before "
- "InitializeConnection()");
+ "Producer invoked CommitData() before InitializeConnection()");
return;
}
- // TODO(fmayer): check that the page indexes are consistent with the size of
- // the shared memory region (once the SHM logic is there). Also add a test for
- // it.
- std::vector<uint32_t> changed_pages;
- changed_pages.reserve(req.changed_pages_size());
- for (const uint32_t& changed_page : req.changed_pages())
- changed_pages.push_back(changed_page);
- producer->service_endpoint->NotifySharedMemoryUpdate(changed_pages);
+ CommitDataRequest req;
+ req.FromProto(proto_req);
+ producer->service_endpoint->CommitData(req);
}
void ProducerIPCService::GetAsyncCommand(
diff --git a/src/tracing/ipc/service/producer_ipc_service.h b/src/tracing/ipc/service/producer_ipc_service.h
index 2fa76a5..1873b44 100644
--- a/src/tracing/ipc/service/producer_ipc_service.h
+++ b/src/tracing/ipc/service/producer_ipc_service.h
@@ -50,9 +50,8 @@
DeferredRegisterDataSourceResponse) override;
void UnregisterDataSource(const protos::UnregisterDataSourceRequest&,
DeferredUnregisterDataSourceResponse) override;
- void NotifySharedMemoryUpdate(
- const protos::NotifySharedMemoryUpdateRequest&,
- DeferredNotifySharedMemoryUpdateResponse) override;
+ void CommitData(const protos::CommitDataRequest&,
+ DeferredCommitDataResponse) override;
void GetAsyncCommand(const protos::GetAsyncCommandRequest&,
DeferredGetAsyncCommandResponse) override;
void OnClientDisconnected() override;
diff --git a/src/tracing/test/tracing_integration_test.cc b/src/tracing/test/tracing_integration_test.cc
index 9709bf7..f79a652 100644
--- a/src/tracing/test/tracing_integration_test.cc
+++ b/src/tracing/test/tracing_integration_test.cc
@@ -169,8 +169,7 @@
writer->NewTracePacket()->set_for_testing()->set_str(buf, strlen(buf));
}
- // Allow the service to see the NotifySharedMemoryUpdate() before disabling
- // tracing.
+ // Allow the service to see the CommitData() before disabling tracing.
task_runner_->RunUntilIdle();
// Disable tracing.
diff --git a/tools/gen_tracing_cpp_headers_from_protos.py b/tools/gen_tracing_cpp_headers_from_protos.py
index 650092f..c12057f 100755
--- a/tools/gen_tracing_cpp_headers_from_protos.py
+++ b/tools/gen_tracing_cpp_headers_from_protos.py
@@ -22,6 +22,7 @@
'perfetto/config/data_source_descriptor.proto',
'perfetto/config/ftrace/ftrace_config.proto',
'perfetto/config/trace_config.proto',
+ 'perfetto/ipc/commit_data_request.proto',
)
HEADER_PATH = 'include/perfetto/tracing/core'