`span`-ify `mojo/.../data_pipe.h`: Migrate `services/tracing`. [chromium/src : main]

0 views
Skip to first unread message

Łukasz Anforowicz (Gerrit)

unread,
Jun 10, 2024, 7:53:51 PMJun 10
to Sami Kyöstilä, chromium...@chromium.org, spang...@chromium.org, wfh+...@chromium.org, blundell+...@chromium.org, tracing...@chromium.org
Attention needed from Sami Kyöstilä

Łukasz Anforowicz has uploaded the change for review

Łukasz Anforowicz would like Sami Kyöstilä to review this change.

Commit message

`span`-ify `mojo/.../data_pipe.h`: Migrate `services/tracing`.

`span`-ification: `mojo::DataPipe[Consumer|Producer]Handle`: Migrating
callers.  This CL `span`-ifies the callers of the APIs in
`mojo/public/cpp/system/data_pipe.h`:

* `BeginReadData` and `BeginWriteData` return (via an out parameter)
  a `span` instead of returning a ptr+size pair.
* `ReadData` and `WriteData` consume a `span` instead of consuming
  a ptr+size pair.

For more details see the design note here:
https://docs.google.com/document/d/1c4NKpXwpQ9MKK1SbJ4C6MvhXI8-KJZ4jq7N4VHTHJoI/edit?usp=sharing

This CL was uploaded by git cl split.

R=skyo...@chromium.org
Bug: 40284755
Change-Id: I457c0d678de7faf7f706043f101fc3af8652443b

Change diff

diff --git a/services/tracing/perfetto/consumer_host.cc b/services/tracing/perfetto/consumer_host.cc
index 0eaa7b15..85e0fca 100644
--- a/services/tracing/perfetto/consumer_host.cc
+++ b/services/tracing/perfetto/consumer_host.cc
@@ -11,6 +11,7 @@
 #include <vector>
 
 #include "base/containers/contains.h"
+#include "base/containers/span.h"
 #include "base/logging.h"
 #include "base/notreached.h"
 #include "base/ranges/algorithm.h"
@@ -105,16 +106,14 @@
   void WriteToStream(std::unique_ptr<Slice> slice, bool has_more) {
     DCHECK(stream_.is_valid());
 
-    size_t write_position = 0;
-    while (write_position < slice->size()) {
-      size_t write_bytes = slice->size() - write_position;
-
-      MojoResult result =
-          stream_->WriteData(slice->data() + write_position, &write_bytes,
-                             MOJO_WRITE_DATA_FLAG_NONE);
+    base::span<const uint8_t> bytes = base::as_byte_span(*slice);
+    while (!bytes.empty()) {
+      size_t actually_written_bytes = 0;
+      MojoResult result = stream_->WriteData(bytes, MOJO_WRITE_DATA_FLAG_NONE,
+                                             actually_written_bytes);
 
       if (result == MOJO_RESULT_OK) {
-        write_position += write_bytes;
+        bytes = bytes.subspan(actually_written_bytes);
         continue;
       }

Change information

Files:
  • M services/tracing/perfetto/consumer_host.cc
Change size: S
Delta: 1 file changed, 7 insertions(+), 8 deletions(-)
Open in Gerrit

Related details

Attention is currently required from:
  • Sami Kyöstilä
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newchange
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I457c0d678de7faf7f706043f101fc3af8652443b
Gerrit-Change-Number: 5618992
Gerrit-PatchSet: 1
Gerrit-Owner: Łukasz Anforowicz <luk...@chromium.org>
Gerrit-Reviewer: Sami Kyöstilä <skyo...@chromium.org>
Gerrit-Reviewer: Łukasz Anforowicz <luk...@chromium.org>
Gerrit-Attention: Sami Kyöstilä <skyo...@chromium.org>
satisfied_requirement
unsatisfied_requirement
open
diffy

Łukasz Anforowicz (Gerrit)

unread,
Jun 11, 2024, 2:21:02 PMJun 11
to Sami Kyöstilä, chromium...@chromium.org, blundell+...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Sami Kyöstilä

Łukasz Anforowicz voted and added 1 comment

Votes added by Łukasz Anforowicz

Auto-Submit+0

1 comment

Patchset-level comments
File-level comment, Patchset 1 (Latest):
Łukasz Anforowicz . resolved

Please note that the prerequisite CL (providing the new API in https://crrev.com/c/5598233) got reverted because of MSAN issues. Therefore before the current-CL-under-review can land, I need to first reland the other CL. But, I think it's still ok to proceed with the review.

Open in Gerrit

Related details

Attention is currently required from:
  • Sami Kyöstilä
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I457c0d678de7faf7f706043f101fc3af8652443b
Gerrit-Change-Number: 5618992
Gerrit-PatchSet: 1
Gerrit-Owner: Łukasz Anforowicz <luk...@chromium.org>
Gerrit-Reviewer: Sami Kyöstilä <skyo...@chromium.org>
Gerrit-Reviewer: Łukasz Anforowicz <luk...@chromium.org>
Gerrit-Attention: Sami Kyöstilä <skyo...@chromium.org>
Gerrit-Comment-Date: Tue, 11 Jun 2024 18:20:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Łukasz Anforowicz (Gerrit)

unread,
Jun 18, 2024, 1:07:15 PM (13 days ago) Jun 18
to Alexander Timin, chromium...@chromium.org, blundell+...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Alexander Timin

Łukasz Anforowicz voted and added 1 comment

Votes added by Łukasz Anforowicz

Auto-Submit+1

1 comment

Patchset-level comments
Łukasz Anforowicz . resolved

@altimin, can you PTAL?

Open in Gerrit

Related details

Attention is currently required from:
  • Alexander Timin
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I457c0d678de7faf7f706043f101fc3af8652443b
Gerrit-Change-Number: 5618992
Gerrit-PatchSet: 1
Gerrit-Owner: Łukasz Anforowicz <luk...@chromium.org>
Gerrit-Reviewer: Alexander Timin <alt...@chromium.org>
Gerrit-Reviewer: Łukasz Anforowicz <luk...@chromium.org>
Gerrit-Attention: Alexander Timin <alt...@chromium.org>
Gerrit-Comment-Date: Tue, 18 Jun 2024 17:07:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Alexander Timin (Gerrit)

unread,
Jun 19, 2024, 7:36:30 AM (12 days ago) Jun 19
to Łukasz Anforowicz, chromium...@chromium.org, blundell+...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Łukasz Anforowicz

Alexander Timin voted

Code-Review+1
Commit-Queue+2
Open in Gerrit

Related details

Attention is currently required from:
  • Łukasz Anforowicz
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I457c0d678de7faf7f706043f101fc3af8652443b
Gerrit-Change-Number: 5618992
Gerrit-PatchSet: 1
Gerrit-Owner: Łukasz Anforowicz <luk...@chromium.org>
Gerrit-Reviewer: Alexander Timin <alt...@chromium.org>
Gerrit-Reviewer: Łukasz Anforowicz <luk...@chromium.org>
Gerrit-Attention: Łukasz Anforowicz <luk...@chromium.org>
Gerrit-Comment-Date: Wed, 19 Jun 2024 11:36:13 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
open
diffy

Łukasz Anforowicz (Gerrit)

unread,
Jun 20, 2024, 1:39:56 PM (11 days ago) Jun 20
to Chromium LUCI CQ, Alexander Timin, chromium...@chromium.org, blundell+...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Alexander Timin

Łukasz Anforowicz voted Commit-Queue+2

Commit-Queue+2
Open in Gerrit

Related details

Attention is currently required from:
  • Alexander Timin
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I457c0d678de7faf7f706043f101fc3af8652443b
Gerrit-Change-Number: 5618992
Gerrit-PatchSet: 1
Gerrit-Owner: Łukasz Anforowicz <luk...@chromium.org>
Gerrit-Reviewer: Alexander Timin <alt...@chromium.org>
Gerrit-Reviewer: Łukasz Anforowicz <luk...@chromium.org>
Gerrit-Attention: Alexander Timin <alt...@chromium.org>
Gerrit-Comment-Date: Thu, 20 Jun 2024 17:39:43 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
open
diffy

Chromium LUCI CQ (Gerrit)

unread,
Jun 20, 2024, 2:46:34 PM (11 days ago) Jun 20
to Łukasz Anforowicz, Alexander Timin, chromium...@chromium.org, blundell+...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:
`span`-ify `mojo/.../data_pipe.h`: Migrate `services/tracing`.

`span`-ification: `mojo::DataPipe[Consumer|Producer]Handle`: Migrating
callers.  This CL `span`-ifies the callers of the APIs in
`mojo/public/cpp/system/data_pipe.h`:

* `BeginReadData` and `BeginWriteData` return (via an out parameter)
  a `span` instead of returning a ptr+size pair.
* `ReadData` and `WriteData` consume a `span` instead of consuming
  a ptr+size pair.

For more details see the design note here:
https://docs.google.com/document/d/1c4NKpXwpQ9MKK1SbJ4C6MvhXI8-KJZ4jq7N4VHTHJoI/edit?usp=sharing

This CL was uploaded by git cl split.

R=skyo...@chromium.org
Bug: 40284755
Change-Id: I457c0d678de7faf7f706043f101fc3af8652443b
Reviewed-by: Alexander Timin <alt...@chromium.org>
Auto-Submit: Łukasz Anforowicz <luk...@chromium.org>
Commit-Queue: Łukasz Anforowicz <luk...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1317528}
Files:
  • M services/tracing/perfetto/consumer_host.cc
Change size: S
Delta: 1 file changed, 7 insertions(+), 8 deletions(-)
Branch: refs/heads/main
Submit Requirements:
  • requirement satisfiedCode-Review: +1 by Alexander Timin
Open in Gerrit
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: merged
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I457c0d678de7faf7f706043f101fc3af8652443b
Gerrit-Change-Number: 5618992
Gerrit-PatchSet: 2
Gerrit-Owner: Łukasz Anforowicz <luk...@chromium.org>
Gerrit-Reviewer: Alexander Timin <alt...@chromium.org>
Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
Gerrit-Reviewer: Łukasz Anforowicz <luk...@chromium.org>
open
diffy
satisfied_requirement
Reply all
Reply to author
Forward
0 new messages