Skip to content

Commit

Permalink
desks: Add animation latency histograms
Browse files Browse the repository at this point in the history
These will measure latency for the desk switch animation to start.

Bug: b/227380117
Test: ash_unittests DesksTest.AnimationLatency*
Change-Id: I49a1e75d8c647ec422d6a077778c89dea5e4613d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3564266
Reviewed-by: Tony Yeoman <tby@chromium.org>
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Avery Musbach <amusbach@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988196}
  • Loading branch information
Avery Musbach authored and Chromium LUCI CQ committed Apr 2, 2022
1 parent 219f4ff commit 7141e8c
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 11 deletions.
9 changes: 8 additions & 1 deletion ash/wm/desks/desk_animation_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,15 @@ DeskAnimationBase::~DeskAnimationBase() {
}

void DeskAnimationBase::Launch() {
launch_time_ = base::TimeTicks::Now();

for (auto& observer : controller_->observers_)
observer.OnDeskSwitchAnimationLaunching();

// The throughput tracker measures the animation when the user lifts their
// fingers off the trackpad, which is done in EndSwipeAnimation.
if (!is_continuous_gesture_animation_)
throughput_tracker_.Start(GetReportCallback());
throughput_tracker_.Start(GetSmoothnessReportCallback());

// This step makes sure that the containers of the target desk are shown at
// the beginning of the animation (but not actually visible to the user yet,
Expand Down Expand Up @@ -122,6 +124,11 @@ void DeskAnimationBase::OnEndingDeskScreenshotTaken() {
if (skip_start_animation)
return;

if (!launch_time_.is_null()) {
GetLatencyReportCallback().Run(base::TimeTicks::Now() - launch_time_);
launch_time_ = base::TimeTicks();
}

for (auto& animator : desk_switch_animators_)
animator->StartAnimation();
}
Expand Down
19 changes: 14 additions & 5 deletions ash/wm/desks/desk_animation_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include "ash/public/cpp/metrics_util.h"
#include "ash/wm/desks/desks_histogram_enums.h"
#include "ash/wm/desks/root_window_desk_switch_animator.h"
#include "base/callback.h"
#include "base/time/time.h"
#include "ui/compositor/throughput_tracker.h"

namespace ash {
Expand Down Expand Up @@ -78,11 +80,13 @@ class ASH_EXPORT DeskAnimationBase
virtual void OnDeskSwitchAnimationFinishedInternal() = 0;

// Since performance here matters, we have to use the UMA histograms macros to
// report the smoothness histograms, but each macro use has to be associated
// with exactly one histogram name. This function allows subclasses to return
// a callback that reports the histogram using the macro with their desired
// name.
virtual metrics_util::ReportCallback GetReportCallback() const = 0;
// report the histograms, but each macro use has to be associated with exactly
// one histogram name. These functions allow subclasses to return callbacks
// that report each histogram using the macro with their desired name.
using LatencyReportCallback =
base::OnceCallback<void(const base::TimeDelta& latency)>;
virtual LatencyReportCallback GetLatencyReportCallback() const = 0;
virtual metrics_util::ReportCallback GetSmoothnessReportCallback() const = 0;

DesksController* const controller_;

Expand Down Expand Up @@ -116,6 +120,11 @@ class ASH_EXPORT DeskAnimationBase
// remove animation.
int visible_desk_changes_ = 0;

// Used for the Ash.Desks.AnimationLatency.* histograms. Null if no animation
// is being prepared. In a continuous desk animation, the latency is reported
// only for the first desk switch, and `launch_time_` is null thereafter.
base::TimeTicks launch_time_;

// ThroughputTracker used for measuring this animation smoothness.
ui::ThroughputTracker throughput_tracker_;

Expand Down
25 changes: 22 additions & 3 deletions ash/wm/desks/desk_animation_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@ namespace ash {

namespace {

constexpr char kDeskActivationLatencyHistogramName[] =
"Ash.Desks.AnimationLatency.DeskActivation";
constexpr char kDeskActivationSmoothnessHistogramName[] =
"Ash.Desks.AnimationSmoothness.DeskActivation";
constexpr char kDeskRemovalLatencyHistogramName[] =
"Ash.Desks.AnimationLatency.DeskRemoval";
constexpr char kDeskRemovalSmoothnessHistogramName[] =
"Ash.Desks.AnimationSmoothness.DeskRemoval";

Expand Down Expand Up @@ -261,8 +265,15 @@ void DeskActivationAnimation::OnDeskSwitchAnimationFinishedInternal() {
update_window_activation_);
}

metrics_util::ReportCallback DeskActivationAnimation::GetReportCallback()
const {
DeskAnimationBase::LatencyReportCallback
DeskActivationAnimation::GetLatencyReportCallback() const {
return base::BindOnce([](const base::TimeDelta& latency) {
UMA_HISTOGRAM_TIMES(kDeskActivationLatencyHistogramName, latency);
});
}

metrics_util::ReportCallback
DeskActivationAnimation::GetSmoothnessReportCallback() const {
return metrics_util::ForSmoothness(base::BindRepeating([](int smoothness) {
UMA_HISTOGRAM_PERCENTAGE(kDeskActivationSmoothnessHistogramName,
smoothness);
Expand Down Expand Up @@ -365,7 +376,15 @@ void DeskRemovalAnimation::OnDeskSwitchAnimationFinishedInternal() {
MaybeRestoreSplitView(/*refresh_snapped_windows=*/true);
}

metrics_util::ReportCallback DeskRemovalAnimation::GetReportCallback() const {
DeskAnimationBase::LatencyReportCallback
DeskRemovalAnimation::GetLatencyReportCallback() const {
return base::BindOnce([](const base::TimeDelta& latency) {
UMA_HISTOGRAM_TIMES(kDeskRemovalLatencyHistogramName, latency);
});
}

metrics_util::ReportCallback DeskRemovalAnimation::GetSmoothnessReportCallback()
const {
return ash::metrics_util::ForSmoothness(
base::BindRepeating([](int smoothness) {
UMA_HISTOGRAM_PERCENTAGE(kDeskRemovalSmoothnessHistogramName,
Expand Down
6 changes: 4 additions & 2 deletions ash/wm/desks/desk_animation_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ class ASH_EXPORT DeskActivationAnimation : public DeskAnimationBase {
bool EndSwipeAnimation() override;
void OnStartingDeskScreenshotTakenInternal(int ending_desk_index) override;
void OnDeskSwitchAnimationFinishedInternal() override;
metrics_util::ReportCallback GetReportCallback() const override;
LatencyReportCallback GetLatencyReportCallback() const override;
metrics_util::ReportCallback GetSmoothnessReportCallback() const override;

private:
FRIEND_TEST_ALL_PREFIXES(DeskActivationAnimationTest,
Expand Down Expand Up @@ -82,7 +83,8 @@ class DeskRemovalAnimation : public DeskAnimationBase {
// DeskAnimationBase:
void OnStartingDeskScreenshotTakenInternal(int ending_desk_index) override;
void OnDeskSwitchAnimationFinishedInternal() override;
metrics_util::ReportCallback GetReportCallback() const override;
LatencyReportCallback GetLatencyReportCallback() const override;
metrics_util::ReportCallback GetSmoothnessReportCallback() const override;

private:
const int desk_to_remove_index_;
Expand Down
24 changes: 24 additions & 0 deletions ash/wm/desks/desks_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1927,6 +1927,30 @@ TEST_F(DesksTest, FullscreenStateUpdatedAcrossDesks) {
EXPECT_TRUE(full_screen_state_observer.is_fullscreen());
}

// Tests the Ash.Desks.AnimationLatency.DeskActivation histogram.
TEST_F(DesksTest, AnimationLatencyDeskActivation) {
NewDesk();
auto* controller = DesksController::Get();
ASSERT_EQ(2u, controller->desks().size());

base::HistogramTester histogram_tester;
ActivateDesk(controller->desks()[1].get());
histogram_tester.ExpectTotalCount("Ash.Desks.AnimationLatency.DeskActivation",
1);
}

// Tests the Ash.Desks.AnimationLatency.DeskRemoval histogram.
TEST_F(DesksTest, AnimationLatencyDeskRemoval) {
NewDesk();
auto* controller = DesksController::Get();
ASSERT_EQ(2u, controller->desks().size());

base::HistogramTester histogram_tester;
RemoveDesk(controller->desks()[0].get());
histogram_tester.ExpectTotalCount("Ash.Desks.AnimationLatency.DeskRemoval",
1);
}

class DesksWithMultiDisplayOverview : public AshTestBase {
public:
DesksWithMultiDisplayOverview() = default;
Expand Down
24 changes: 24 additions & 0 deletions tools/metrics/histograms/metadata/ash/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1172,6 +1172,30 @@ chromium-metrics-reviews@google.com.
</summary>
</histogram>

<histogram name="Ash.Desks.AnimationLatency.DeskActivation" units="ms"
expires_after="2023-04-23">
<owner>amusbach@chromium.org</owner>
<owner>afakhry@chromium.org</owner>
<owner>tclaiborne@chromium.org</owner>
<summary>
Emitted when the virtual desks activation animation begins, to report the
latency of starting this animation. In a continuous desk animation, this
metric is recorded only for the first desk switch.
</summary>
</histogram>

<histogram name="Ash.Desks.AnimationLatency.DeskRemoval" units="ms"
expires_after="2023-04-23">
<owner>amusbach@chromium.org</owner>
<owner>afakhry@chromium.org</owner>
<owner>tclaiborne@chromium.org</owner>
<summary>
Emitted when the virtual desks removal animation begins, to report the
latency of starting this animation. In a continuous desk animation, this
metric is recorded only for the first desk switch.
</summary>
</histogram>

<histogram name="Ash.Desks.AnimationSmoothness.DeskActivation" units="%"
expires_after="2023-04-23">
<owner>afakhry@chromium.org</owner>
Expand Down

0 comments on commit 7141e8c

Please sign in to comment.