0

Unrewrite raw_ptr/raw_ref for performance reasons (CrOS)

This is based on profiling data:
https://pprof.corp.google.com/user-profile?id=102df5c4bcfd3ec267cc1cae0793757e&id0=222c6842aa2283ac2ce7945f99416270&tab=bottomup

This CL unrewrites top 5 hits of AcquireInternal and top 3 of
ReleaseInternal.

Bug: 1455207
Change-Id: I761d9c54654a24f512a7f053847ea47f2e020404
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4622499
Owners-Override: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Keishi Hattori <keishi@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Auto-Submit: Bartek Nowierski <bartekn@chromium.org>
Commit-Queue: Bartek Nowierski <bartekn@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1160181}
This commit is contained in:
Bartek Nowierski
2023-06-20 19:00:10 +00:00
committed by Chromium LUCI CQ
parent 5a2f1efdc6
commit 0201188b0e
10 changed files with 63 additions and 31 deletions

@ -10,7 +10,7 @@
#include <iterator>
#include <utility>
#include "base/memory/raw_ref.h"
#include "base/memory/raw_ptr_exclusion.h"
namespace base {
@ -26,15 +26,14 @@ class ReversedAdapter {
ReversedAdapter(const ReversedAdapter& ra) : t_(ra.t_) {}
ReversedAdapter& operator=(const ReversedAdapter&) = delete;
Iterator begin() const { return std::rbegin(*t_); }
Iterator end() const { return std::rend(*t_); }
Iterator begin() const { return std::rbegin(t_); }
Iterator end() const { return std::rend(t_); }
private:
// `ReversedAdapter` and therefore `t_` are only used inside for loops. The
// container being iterated over should be the one holding a raw_ref/raw_ptr
// ideally. This member's type was rewritten into `const raw_ref` since it
// didn't hurt binary size at the time of the rewrite.
const raw_ref<T> t_;
// Not a raw_ref<...> for performance reasons: on-stack pointer.
// It is only used inside for loops. Ideally, the container being iterated
// over should be the one held via a raw_ref/raw_ptrs.
RAW_PTR_EXCLUSION T& t_;
};
} // namespace internal

@ -327,7 +327,7 @@ TaskAnnotator::LongTaskTracker::~LongTaskTracker() {
TRACE_EVENT_BEGIN("scheduler.long_tasks", "LongTaskTracker",
perfetto::Track::ThreadScoped(task_annotator_),
task_start_time_, [&](perfetto::EventContext& ctx) {
TaskAnnotator::EmitTaskLocation(ctx, *pending_task_);
TaskAnnotator::EmitTaskLocation(ctx, pending_task_);
EmitReceivedIPCDetails(ctx);
});
TRACE_EVENT_END("scheduler.long_tasks",
@ -386,9 +386,9 @@ void TaskAnnotator::LongTaskTracker::MaybeTraceInterestingTaskDetails() {
// start of the flow between task queue time and task execution start time.
TRACE_EVENT_INSTANT("scheduler.long_tasks", "InterestingTask_QueueingTime",
perfetto::Track::ThreadScoped(task_annotator_),
pending_task_->queue_time,
pending_task_.queue_time,
perfetto::Flow::ProcessScoped(
task_annotator_->GetTaskTraceID(*pending_task_)));
task_annotator_->GetTaskTraceID(pending_task_)));
// Record the equivalent of a top-level event with enough IPC information
// to calculate the input to browser interval. This event will be the
@ -398,7 +398,7 @@ void TaskAnnotator::LongTaskTracker::MaybeTraceInterestingTaskDetails() {
perfetto::Track::ThreadScoped(task_annotator_), task_start_time_,
[&](perfetto::EventContext& ctx) {
perfetto::TerminatingFlow::ProcessScoped(
task_annotator_->GetTaskTraceID(*pending_task_))(ctx);
task_annotator_->GetTaskTraceID(pending_task_))(ctx);
auto* info = ctx.event()->set_chrome_mojo_event_info();
info->set_mojo_interface_tag(ipc_interface_name_);
});

@ -9,8 +9,7 @@
#include "base/auto_reset.h"
#include "base/base_export.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ref.h"
#include "base/memory/raw_ptr_exclusion.h"
#include "base/pending_task.h"
#include "base/strings/string_piece.h"
#include "base/time/tick_clock.h"
@ -170,8 +169,11 @@ class BASE_EXPORT [[maybe_unused, nodiscard]] TaskAnnotator::LongTaskTracker {
const AutoReset<LongTaskTracker*> resetter_;
// For tracking task duration
raw_ptr<const TickClock> tick_clock_; // Not owned.
// For tracking task duration.
//
// Not a raw_ptr<...> for performance reasons: based on analysis of sampling
// profiler data (TaskAnnotator::LongTaskTracker::~LongTaskTracker).
RAW_PTR_EXCLUSION const TickClock* tick_clock_; // Not owned.
TimeTicks task_start_time_;
TimeTicks task_end_time_;
@ -187,8 +189,10 @@ class BASE_EXPORT [[maybe_unused, nodiscard]] TaskAnnotator::LongTaskTracker {
// known. Note that this will not compile in the Native client.
uint32_t (*ipc_method_info_)();
bool is_response_ = false;
[[maybe_unused]] const raw_ref<PendingTask> pending_task_;
[[maybe_unused]] raw_ptr<TaskAnnotator> task_annotator_;
// Not a raw_ptr/raw_ref<...> for performance reasons: based on analysis of
// sampling profiler data (TaskAnnotator::LongTaskTracker::~LongTaskTracker).
[[maybe_unused]] RAW_PTR_EXCLUSION PendingTask& pending_task_;
[[maybe_unused]] RAW_PTR_EXCLUSION TaskAnnotator* task_annotator_;
};
} // namespace base

@ -13,6 +13,7 @@
#include <vector>
#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ptr_exclusion.h"
#include "cc/base/region.h"
#include "cc/tiles/picture_layer_tiling.h"
#include "ui/gfx/geometry/size.h"
@ -211,7 +212,11 @@ class CC_EXPORT PictureLayerTilingSet {
~AutoClear() { *state_to_clear_ = StateSinceLastTilePriorityUpdate(); }
private:
raw_ptr<StateSinceLastTilePriorityUpdate> state_to_clear_;
// Not a raw_ptr<...> for performance reasons: on-stack pointer +
// based on analysis of sampling profiler data
// (cc::PictureLayerTilingSet::UpdateTilePriorities -> creates AutoClear
// on stack).
RAW_PTR_EXCLUSION StateSinceLastTilePriorityUpdate* state_to_clear_;
};
StateSinceLastTilePriorityUpdate()

@ -25,7 +25,7 @@ Tile::Tile(TileManager* tile_manager,
int source_frame_number,
int flags)
: tile_manager_(tile_manager),
tiling_(info.tiling.get()),
tiling_(info.tiling),
content_rect_(info.content_rect),
enclosing_layer_rect_(info.enclosing_layer_rect),
raster_transform_(info.raster_transform),

@ -12,7 +12,7 @@
#include <utility>
#include <vector>
#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ptr_exclusion.h"
#include "base/memory/ref_counted.h"
#include "cc/paint/draw_image.h"
#include "cc/raster/tile_task.h"
@ -29,7 +29,12 @@ class TileManager;
class CC_EXPORT Tile {
public:
struct CreateInfo {
raw_ptr<const PictureLayerTiling> tiling = nullptr;
// Not a raw_ptr<...> for performance reasons: on-stack pointer + based on
// analysis of sampling profiler data
// (PictureLayerTilingSet::UpdateTilePriorities ->
// PictureLayerTiling::ComputeTilePriorityRects ->
// PictureLayerTiling::SetLiveTilesRect -> creates Tile::CreateInfo).
RAW_PTR_EXCLUSION const PictureLayerTiling* tiling = nullptr;
int tiling_i_index = 0;
int tiling_j_index = 0;
gfx::Rect enclosing_layer_rect;
@ -146,8 +151,14 @@ class CC_EXPORT Tile {
int source_frame_number,
int flags);
const raw_ptr<TileManager> tile_manager_;
raw_ptr<const PictureLayerTiling> tiling_;
// These are not a raw_ptr<...> for performance reasons: based on analysis of
// sampling profiler data (PictureLayerTilingSet::UpdateTilePriorities ->
// PictureLayerTiling::ComputeTilePriorityRects ->
// PictureLayerTiling::SetLiveTilesRect -> PictureLayerTiling::CreateTile ->
// allocates Tile).
RAW_PTR_EXCLUSION TileManager* const tile_manager_;
RAW_PTR_EXCLUSION const PictureLayerTiling* tiling_;
const gfx::Rect content_rect_;
const gfx::Rect enclosing_layer_rect_;
const gfx::AxisTransform2d raster_transform_;

@ -7,7 +7,7 @@
#include <vector>
#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ptr_exclusion.h"
#include "cc/base/simple_enclosed_region.h"
#include "cc/cc_export.h"
#include "cc/layers/effect_tree_layer_list_iterator.h"
@ -62,7 +62,13 @@ class CC_EXPORT OcclusionTracker {
struct StackObject {
StackObject() : target(nullptr) {}
explicit StackObject(const RenderSurfaceImpl* target) : target(target) {}
raw_ptr<const RenderSurfaceImpl, DanglingUntriaged> target;
// Not a raw_ptr<...> for performance reasons: on-stack (temporary) vector +
// based on analysis of sampling profiler data
// (LayerTreeImpl::UpdateDrawProperties -> OcclusionTracker::EnterLayer ->
// OcclusionTracker::EnterRenderTarget -> emplaces StackObject in a vector;
// ... -> OcclusionTracker::LeaveLayer ->
// OcclusionTracker::LeaveToRenderTarget -> pops StackObject from a vector).
RAW_PTR_EXCLUSION const RenderSurfaceImpl* target;
SimpleEnclosedRegion occlusion_from_outside_target;
SimpleEnclosedRegion occlusion_from_inside_target;
bool ignores_parent_occlusion = false;

@ -316,7 +316,7 @@ JankMonitorImpl::ThreadExecutionState::CheckJankiness() {
}
// Mark that the target thread is janky and notify the monitor thread.
return task_execution_metadata_.back().identifier.get();
return task_execution_metadata_.back().identifier;
}
void JankMonitorImpl::ThreadExecutionState::WillRunTaskOrEvent(

@ -8,7 +8,7 @@
#include <atomic>
#include "base/gtest_prod_util.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ptr_exclusion.h"
#include "base/observer_list.h"
#include "base/sequence_checker.h"
#include "base/synchronization/lock.h"
@ -88,7 +88,11 @@ class CONTENT_EXPORT JankMonitorImpl : public content::JankMonitor,
~TaskMetadata();
base::TimeTicks execution_start_time;
raw_ptr<const void, DanglingUntriaged> identifier;
// Not a raw_ptr<...> for performance reasons: based on analysis of
// sampling profiler data (JankMonitorImpl::WillRunTaskOrEvent ->
// JankMonitorImpl::ThreadExecutionState::WillRunTaskOrEvent -> emplaces
// TaskMetadata in a vector).
RAW_PTR_EXCLUSION const void* identifier;
};
std::vector<TaskMetadata> task_execution_metadata_;

@ -10,7 +10,6 @@
#include <string>
#include "base/component_export.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ptr_exclusion.h"
#include "mojo/public/cpp/bindings/lib/bindings_internal.h"
@ -162,7 +161,11 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS_BASE) ValidationContext {
return end > begin && begin >= data_begin_ && end <= data_end_;
}
const raw_ptr<Message> message_;
// Not a raw_ptr<...> for performance reasons: on-stack pointer + based on
// analysis of sampling profiler data (MultiplexRouter::ProcessIncomingMessage
// -> PipeControlMessageHandler::Accept -> PipeControlMessageHandler::Validate
// -> constructs ValidationContext).
RAW_PTR_EXCLUSION Message* const message_;
const char* const description_;
const ValidatorType validator_type_;