Skip to content

Commit

Permalink
[BRP] Unrewrite fields based on Mac sampling profiler data
Browse files Browse the repository at this point in the history
This CL unrewrites the top three call sites for RawPtrBackupRefImpl::
AcquireInternal based on Mac sampling profiler data
go/brp-mac-prof-diff-20230403

(cherry picked from commit 8e7c8fc)

Bug: 1431045
Change-Id: Ib0d1c6c5671456bc4cea0538cdf9ee8a03b82f7e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4393619
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Keishi Hattori <keishi@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1127015}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4422254
Cr-Commit-Position: refs/branch-heads/5672@{#673}
Cr-Branched-From: 5f2a724-refs/heads/main@{#1121455}
  • Loading branch information
Keishi Hattori authored and Chromium LUCI CQ committed Apr 18, 2023
1 parent 99f9a44 commit 7362de1
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 14 deletions.
7 changes: 4 additions & 3 deletions base/task/sequence_manager/sequenced_task_source.h
Expand Up @@ -7,7 +7,7 @@

#include "base/base_export.h"
#include "base/functional/callback_helpers.h"
#include "base/memory/raw_ref.h"
#include "base/memory/raw_ptr_exclusion.h"
#include "base/pending_task.h"
#include "base/task/common/lazy_now.h"
#include "base/task/sequence_manager/task_queue.h"
Expand Down Expand Up @@ -38,8 +38,9 @@ class SequencedTaskSource {
QueueName task_queue_name);
~SelectedTask();

// TODO(crbug.com/1409100): breaks base_unittests.
const raw_ref<Task, DisableDanglingPtrDetection> task;
// `task` is not a raw_ref<> for performance reasons: based on this sampling
// profiler result on Mac. go/brp-mac-prof-diff-20230403
RAW_PTR_EXCLUSION Task& task;
// Callback to fill trace event arguments associated with the task
// execution. Can be null
TaskExecutionTraceLogger task_execution_trace_logger =
Expand Down
8 changes: 4 additions & 4 deletions base/task/sequence_manager/thread_controller_impl.cc
Expand Up @@ -192,8 +192,8 @@ void ThreadControllerImpl::DoWork(WorkType work_type) {
sequence_->SelectNextTask(lazy_now_select_task);
LazyNow lazy_now_task_selected(time_source_);
run_level_tracker_.OnApplicationTaskSelected(
(selected_task && selected_task->task->delayed_run_time.is_null())
? selected_task->task->queue_time
(selected_task && selected_task->task.delayed_run_time.is_null())
? selected_task->task.queue_time
: TimeTicks(),
lazy_now_task_selected);
if (!selected_task) {
Expand All @@ -211,11 +211,11 @@ void ThreadControllerImpl::DoWork(WorkType work_type) {
// logging so lambda captures are safe as lambda is executed inline.
SequencedTaskSource* source = sequence_;
task_annotator_.RunTask(
"ThreadControllerImpl::RunTask", *selected_task->task,
"ThreadControllerImpl::RunTask", selected_task->task,
[&selected_task, &source](perfetto::EventContext& ctx) {
if (selected_task->task_execution_trace_logger)
selected_task->task_execution_trace_logger.Run(
ctx, *selected_task->task);
ctx, selected_task->task);
source->MaybeEmitTaskDetails(ctx, *selected_task);
});
if (!weak_ptr)
Expand Down
Expand Up @@ -447,8 +447,8 @@ absl::optional<WakeUp> ThreadControllerWithMessagePumpImpl::DoWorkImpl(
select_task_option);
LazyNow lazy_now_task_selected(time_source_);
run_level_tracker_.OnApplicationTaskSelected(
(selected_task && selected_task->task->delayed_run_time.is_null())
? selected_task->task->queue_time
(selected_task && selected_task->task.delayed_run_time.is_null())
? selected_task->task.queue_time
: TimeTicks(),
lazy_now_task_selected);
if (!selected_task) {
Expand All @@ -468,17 +468,17 @@ absl::optional<WakeUp> ThreadControllerWithMessagePumpImpl::DoWorkImpl(
{
// Always track the start of the task, as this is low-overhead.
TaskAnnotator::LongTaskTracker long_task_tracker(
time_source_, *selected_task->task, &task_annotator_);
time_source_, selected_task->task, &task_annotator_);

// Note: all arguments after task are just passed to a TRACE_EVENT for
// logging so lambda captures are safe as lambda is executed inline.
SequencedTaskSource* source = main_thread_only().task_source;
task_annotator_.RunTask(
"ThreadControllerImpl::RunTask", *selected_task->task,
"ThreadControllerImpl::RunTask", selected_task->task,
[&selected_task, &source](perfetto::EventContext& ctx) {
if (selected_task->task_execution_trace_logger)
selected_task->task_execution_trace_logger.Run(
ctx, *selected_task->task);
ctx, selected_task->task);
source->MaybeEmitTaskDetails(ctx, selected_task.value());
});
}
Expand Down
6 changes: 4 additions & 2 deletions mojo/public/cpp/bindings/string_data_view.h
Expand Up @@ -5,7 +5,7 @@
#ifndef MOJO_PUBLIC_CPP_BINDINGS_STRING_DATA_VIEW_H_
#define MOJO_PUBLIC_CPP_BINDINGS_STRING_DATA_VIEW_H_

#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ptr_exclusion.h"
#include "mojo/public/cpp/bindings/lib/array_internal.h"

namespace mojo {
Expand All @@ -26,7 +26,9 @@ class StringDataView {
size_t size() const { return data_->size(); }

private:
raw_ptr<internal::String_Data> data_ = nullptr;
// `data_` is not a raw_ptr<> for performance reasons: based on this sampling
// profiler result on Mac. go/brp-mac-prof-diff-20230403
RAW_PTR_EXCLUSION internal::String_Data* data_ = nullptr;
};

} // namespace mojo
Expand Down

0 comments on commit 7362de1

Please sign in to comment.