Skip to content

GitLab

T
tangled
Commit 772b2bcd authored by Christian Biesinger's avatar Christian Biesinger Committed by Commit Bot
Browse files
Options

Revert "Add UKM metrics for rendering timing measurements" 

This reverts commit a324bac1.

Reason for revert: Causes various performance regressions and at least one memory regression, see bugs listed below.

Original change's description:
> Add UKM metrics for rendering timing measurements
>
> UKM review: https://docs.google.com/document/d/1Cpe5kLi3tVshH3evoJVUVDjOw7le-f8jFwBjEk2uadY/edit
>
> Bug: 783383
> Change-Id: Ib684e9e07322e27633b33576c688fa699fada81e
> Reviewed-on: https://chromium-review.googlesource.com/812154
> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#532051}

TBR=cbiesinger@chromium.org,asvitkine@chromium.org,chrishtr@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 783383,807121,806814,806810,806808,806806,806795,806801,806809,806812,806830,806829,806805,806796
Change-Id: I33e40eb337bb0b1daea4be753878d7218342942f
Reviewed-on: https://chromium-review.googlesource.com/892482
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532789}
parent 6c31f3d1
Hide whitespace changes
Inline Side-by-side
Showing with 4 additions and 87 deletions
services/metrics/public/cpp/ukm_recorder.h
View file @ 772b2bcd
......@@ -40,7 +40,6 @@ class BasePredictor;
namespace blink {
class AutoplayUmaHelper;
class Document;
class LocalFrameView;
}
namespace cc {
......@@ -148,7 +147,6 @@ class METRICS_EXPORT UkmRecorder {
friend previews::PreviewsUKMObserver;
friend translate::TranslateRankerImpl;
friend ui::LatencyTracker;
friend blink::LocalFrameView;
FRIEND_TEST_ALL_PREFIXES(UkmServiceTest, AddEntryWithEmptyMetrics);
FRIEND_TEST_ALL_PREFIXES(UkmServiceTest, EntryBuilderAndSerialization);
FRIEND_TEST_ALL_PREFIXES(UkmServiceTest,
......
third_party/WebKit/Source/core/frame/LocalFrameView.cpp
View file @ 772b2bcd
......@@ -128,9 +128,6 @@
#include "public/platform/WebDisplayItemList.h"
#include "public/platform/WebRect.h"
#include "public/platform/WebScrollIntoViewParams.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "services/metrics/public/cpp/ukm_entry_builder.h"
#include "services/metrics/public/cpp/ukm_recorder.h"
#include "third_party/WebKit/common/page/page_visibility_state.mojom-blink.h"
// Used to check for dirty layouts violating document lifecycle rules.
......@@ -156,42 +153,6 @@ constexpr int kLetterPortraitPageHeight = 792;
} // namespace
namespace blink {
namespace {
class ScopedUsHistogramAndUkmTimer {
STACK_ALLOCATED();
public:
explicit ScopedUsHistogramAndUkmTimer(LocalFrameView* view,
CustomCountHistogram& counter,
const char* ukm_metric)
: view_(view),
start_time_(CurrentTimeTicks()),
counter_(&counter),
ukm_metric_(ukm_metric) {}
~ScopedUsHistogramAndUkmTimer() {
int64_t elapsed = (CurrentTimeTicks() - start_time_).InMicroseconds();
counter_->Count(elapsed);
view_->RecordUkmPerformanceMetric(ukm_metric_, elapsed);
}
private:
Member<LocalFrameView> view_;
const TimeTicks start_time_;
CustomCountHistogram* counter_;
const char* ukm_metric_;
DISALLOW_COPY_AND_ASSIGN(ScopedUsHistogramAndUkmTimer);
};
#define SCOPED_UMA_AND_UKM_TIMER(uma_name, ukm_name) \
DEFINE_STATIC_LOCAL_IMPL(CustomCountHistogram, scoped_us_counter, \
(uma_name, 0, 10000000, 50), false); \
ScopedUsHistogramAndUkmTimer timer(this, scoped_us_counter, ukm_name);
} // namespace
using namespace HTMLNames;
// The maximum number of updatePlugins iterations that should be done before
......@@ -3327,7 +3288,7 @@ void LocalFrameView::PrePaint() {
});
{
SCOPED_UMA_AND_UKM_TIMER("Blink.PrePaint.UpdateTime", "PrePaint");
SCOPED_BLINK_UMA_HISTOGRAM_TIMER("Blink.PrePaint.UpdateTime");
PrePaintTreeWalk().Walk(*this);
}
......@@ -3338,7 +3299,7 @@ void LocalFrameView::PrePaint() {
void LocalFrameView::PaintTree() {
TRACE_EVENT0("blink", "LocalFrameView::paintTree");
SCOPED_UMA_AND_UKM_TIMER("Blink.Paint.UpdateTime", "Paint");
SCOPED_BLINK_UMA_HISTOGRAM_TIMER("Blink.Paint.UpdateTime");
DCHECK(GetFrame() == GetPage()->MainFrame() ||
(!GetFrame().Tree().Parent()->IsLocalFrame()));
......@@ -3421,7 +3382,7 @@ void LocalFrameView::PushPaintArtifactToCompositor(
paint_artifact_compositor_->GetWebLayer(), &GetFrame());
}
SCOPED_UMA_AND_UKM_TIMER("Blink.Compositing.UpdateTime", "Compositing");
SCOPED_BLINK_UMA_HISTOGRAM_TIMER("Blink.Compositing.UpdateTime");
paint_artifact_compositor_->Update(paint_controller_->GetPaintArtifact(),
composited_element_ids);
......@@ -3436,7 +3397,7 @@ std::unique_ptr<JSONObject> LocalFrameView::CompositedLayersAsJSON(
}
void LocalFrameView::UpdateStyleAndLayoutIfNeededRecursive() {
SCOPED_UMA_AND_UKM_TIMER("Blink.StyleAndLayout.UpdateTime", "StyleAndLayout");
SCOPED_BLINK_UMA_HISTOGRAM_TIMER("Blink.StyleAndLayout.UpdateTime");
UpdateStyleAndLayoutIfNeededRecursiveInternal();
}
......@@ -5775,14 +5736,4 @@ ScrollbarTheme& LocalFrameView::GetPageScrollbarTheme() const {
return page->GetScrollbarTheme();
}
void LocalFrameView::RecordUkmPerformanceMetric(const char* metric_name,
int64_t value) {
ukm::UkmRecorder* ukm_recorder = frame_->GetDocument()->UkmRecorder();
DCHECK(ukm_recorder);
std::unique_ptr<ukm::UkmEntryBuilder> builder = ukm_recorder->GetEntryBuilder(
frame_->GetDocument()->UkmSourceID(), "Blink.UpdateTime");
builder->AddMetric(metric_name, value);
}
} // namespace blink
third_party/WebKit/Source/core/frame/LocalFrameView.h
View file @ 772b2bcd
......@@ -968,11 +968,6 @@ class CORE_EXPORT LocalFrameView final
// scrollable area, or gains/loses a composited layer.
void ScrollableAreasDidChange();
// Records a UKM metric for the current URL. metric_name is a metric
// that must be defined in ukm.xml; metric_value is in usec.
void RecordUkmPerformanceMetric(const char* metric_name,
int64_t metric_value);
protected:
// Scroll the content via the compositor.
bool ScrollContentsFastPath(const IntSize& scroll_delta);
......
tools/metrics/ukm/ukm.xml
View file @ 772b2bcd
......@@ -2695,31 +2695,4 @@ be describing additional metrics about the same event.
</metric>
</event>
<event name="Blink.UpdateTime">
<owner>chrishtr@chromium.org</owner>
<summary>
Records various rendering performance metrics (in microseconds).
</summary>
<metric name="Compositing">
<summary>
Time taken by the compositing phase in microseconds.
</summary>
</metric>
<metric name="Paint">
<summary>
Time taken by the compositing phase in microseconds.
</summary>
</metric>
<metric name="PrePaint">
<summary>
Time taken by the pre-paint phase in microseconds.
</summary>
</metric>
<metric name="StyleAndLayout">
<summary>
Time taken by the style and layout phases in microseconds.
</summary>
</metric>
</event>
</ukm-configuration>
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment