Skip to content

Commit

Permalink
[IntersectionOptimization] Accumulate scroll delta per IntersectionOb…
Browse files Browse the repository at this point in the history 
…servation

UMA metrics added in crrev.com/1193840 show that most
IntersectionObservations have big min_scroll_delta_to_update, but
most LocalFrameViews have very small min_scroll_delta_to_update. This
means that a few IntersectionObservations with small
min_scroll_delta_to_update cause the optimization not effective.

Now track accumulated scroll delta per IntersectionObservation so that
we can still skip update for a IntersectionObservation with a big
min_scroll_delta_to_update despite other IntersectionObservations with
small min_scroll_delta_to_update.

Bug: 1400495
Change-Id: I69ec0f80248a9fb8b1120d2d84df703c5e6f6597
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4906672
Reviewed-by: Stefan Zager <szager@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1226578}
  • Loading branch information
@wangxianzhu
wangxianzhu authored and Chromium LUCI CQ committed Nov 19, 2023
1 parent b318a28 commit 339bcd9
Show file tree
Hide file tree
Showing 18 changed files with 278 additions and 386 deletions.
17 changes: 5 additions & 12 deletions third_party/blink/renderer/core/frame/frame_view.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,17 @@ bool FrameView::DisplayLockedInParentFrame() {
return DisplayLockUtilities::LockedInclusiveAncestorPreventingPaint(*owner);
}

gfx::Vector2dF FrameView::UpdateViewportIntersection(
unsigned flags,
bool needs_occlusion_tracking) {
if (!(flags &
IntersectionObservation::kFrameViewportIntersectionNeedsUpdate)) {
return min_scroll_delta_to_update_viewport_intersection_;
void FrameView::UpdateViewportIntersection(unsigned flags,
bool needs_occlusion_tracking) {
if (!(flags & IntersectionObservation::kImplicitRootObserversNeedUpdate)) {
return;
}

// This should only run in child frames.
Frame& frame = GetFrame();
HTMLFrameOwnerElement* owner_element = frame.DeprecatedLocalOwner();
if (!owner_element) {
return gfx::Vector2dF();
return;
}

Document& owner_document = owner_element->GetDocument();
Expand All @@ -98,8 +96,6 @@ gfx::Vector2dF FrameView::UpdateViewportIntersection(
// zero size, or it's display locked in parent frame; leave
// viewport_intersection empty, and signal the frame as occluded if
// necessary.
min_scroll_delta_to_update_viewport_intersection_ =
IntersectionGeometry::kInfiniteScrollDelta;
occlusion_state = mojom::blink::FrameOcclusionState::kPossiblyOccluded;
} else if (parent_lifecycle_state >= DocumentLifecycle::kLayoutClean &&
!owner_document.View()->NeedsLayout()) {
Expand All @@ -117,8 +113,6 @@ gfx::Vector2dF FrameView::UpdateViewportIntersection(
/* target_margin */ {},
/* scroll_margin */ {}, geometry_flags, root_geometry);

min_scroll_delta_to_update_viewport_intersection_ =
geometry.MinScrollDeltaToUpdate();
PhysicalRect new_rect_in_parent = geometry.IntersectionRect();

// Convert to DIP
Expand Down Expand Up @@ -295,7 +289,6 @@ gfx::Vector2dF FrameView::UpdateViewportIntersection(
}
UpdateRenderThrottlingStatus(should_throttle, subtree_throttled,
display_locked_in_parent_frame);
return min_scroll_delta_to_update_viewport_intersection_;
}

void FrameView::UpdateFrameVisibility(bool intersects_viewport) {
Expand Down
20 changes: 5 additions & 15 deletions third_party/blink/renderer/core/frame/frame_view.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
namespace blink {

class Frame;
struct IntersectionUpdateResult;
struct IntrinsicSizingInfo;

class CORE_EXPORT FrameView : public EmbeddedContentView {
Expand All @@ -28,7 +27,9 @@ class CORE_EXPORT FrameView : public EmbeddedContentView {
// parent_flags is the result of calling GetIntersectionObservationFlags on
// the LocalFrameView parent of this FrameView (if any). It contains dirty
// bits based on whether geometry may have changed in the parent frame.
virtual IntersectionUpdateResult UpdateViewportIntersectionsForSubtree(
// Returns true if the frame needs occlusion tracking (i.e. trackVisibility()
// is true for any tracked observer in the frame subtree).
virtual bool UpdateViewportIntersectionsForSubtree(
unsigned parent_flags,
absl::optional<base::TimeTicks>& monotonic_time) = 0;

Expand Down Expand Up @@ -78,17 +79,8 @@ class CORE_EXPORT FrameView : public EmbeddedContentView {
const mojom::blink::ViewportIntersectionState& intersection_state) = 0;
virtual void VisibilityForThrottlingChanged() = 0;
virtual bool LifecycleUpdatesThrottled() const { return false; }

// Returns the minimum scroll delta in the parent frame to update
// implicit-root intersection observers in this frame. This only affects
// when the parent frame propagates the kImplicitRootObserversNeedUpdate flag
// to this frame during UpdateViewportIntersectionForSubtree(), but doesn't
// affect the kFrameViewportIntersectionNeedsUpdate flag. The return value
// is only based on the intersection relationship between this frame's
// content rect and the viewport. The caller may disregard the result due to
// other constraints.
gfx::Vector2dF UpdateViewportIntersection(unsigned flags,
bool needs_occlusion_tracking);
void UpdateViewportIntersection(unsigned flags,
bool needs_occlusion_tracking);

// FrameVisibility is tracked by the browser process, which may suppress
// lifecycle updates for a frame outside the viewport.
Expand All @@ -104,8 +96,6 @@ class CORE_EXPORT FrameView : public EmbeddedContentView {
base::TimeTicks rect_in_parent_stable_since_;
base::TimeTicks rect_in_parent_stable_since_for_iov2_;
blink::mojom::FrameVisibility frame_visibility_;
// Caches the result of UpdateVIewportIntersection().
gfx::Vector2dF min_scroll_delta_to_update_viewport_intersection_;
bool hidden_for_throttling_ = false;
bool subtree_throttled_ = false;
bool display_locked_ = false;
Expand Down
9 changes: 6 additions & 3 deletions third_party/blink/renderer/core/frame/local_frame_ukm_aggregator_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/metrics/document_update_reason.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_intersection_observer_init.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_union_document_element.h"
#include "third_party/blink/renderer/core/html/html_frame_owner_element.h"
#include "third_party/blink/renderer/core/paint/timing/paint_timing.h"
#include "third_party/blink/renderer/core/testing/intersection_observer_test_helper.h"
Expand Down Expand Up @@ -691,6 +692,8 @@ class LocalFrameUkmAggregatorSimTest : public SimTest {
// Create internal observer
IntersectionObserverInit* observer_init =
IntersectionObserverInit::Create();
observer_init->setRoot(
MakeGarbageCollected<V8UnionDocumentOrElement>(&document));
TestIntersectionObserverDelegate* internal_delegate =
MakeGarbageCollected<TestIntersectionObserverDelegate>(
document, LocalFrameUkmAggregator::kLazyLoadIntersectionObserver);
Expand Down Expand Up @@ -720,7 +723,7 @@ class LocalFrameUkmAggregatorSimTest : public SimTest {
EXPECT_EQ(
histogram_tester.GetTotalSum(
"Blink.IntersectionObservationInternalCount.UpdateTime.PreFCP"),
4);
RuntimeEnabledFeatures::IntersectionOptimizationEnabled() ? 2 : 4);
EXPECT_EQ(
histogram_tester.GetTotalSum(
"Blink.IntersectionObservationJavascriptCount.UpdateTime.PreFCP"),
Expand All @@ -734,12 +737,12 @@ class LocalFrameUkmAggregatorSimTest : public SimTest {
base::TimeTicks(), base::TimeTicks() + base::Microseconds(10), 0,
root_document->UkmSourceID(), root_document->UkmRecorder());

target1->setAttribute(html_names::kStyleAttr, AtomicString("width: 60px"));
target1->setAttribute(html_names::kStyleAttr, AtomicString("height: 60px"));
Compositor().BeginFrame();
EXPECT_EQ(
histogram_tester.GetTotalSum(
"Blink.IntersectionObservationInternalCount.UpdateTime.PreFCP"),
4);
RuntimeEnabledFeatures::IntersectionOptimizationEnabled() ? 2 : 4);
EXPECT_EQ(
histogram_tester.GetTotalSum(
"Blink.IntersectionObservationJavascriptCount.UpdateTime.PreFCP"),
Expand Down
118 changes: 26 additions & 92 deletions third_party/blink/renderer/core/frame/local_frame_view.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1048,10 +1048,10 @@ void LocalFrameView::RunIntersectionObserverSteps() {
// Populating monotonic_time may be expensive, and may be unnecessary, so
// allow it to be populated on demand.
absl::optional<base::TimeTicks> monotonic_time;
IntersectionUpdateResult result =
bool needs_occlusion_tracking =
UpdateViewportIntersectionsForSubtree(0, monotonic_time);
if (FrameOwner* owner = frame_->Owner())
owner->SetNeedsOcclusionTracking(result.needs_occlusion_tracking);
owner->SetNeedsOcclusionTracking(needs_occlusion_tracking);
#if DCHECK_IS_ON()
DCHECK(was_dirty || !NeedsLayout());
#endif
Expand All @@ -1067,8 +1067,7 @@ void LocalFrameView::ForceUpdateViewportIntersections() {
DocumentUpdateReason::kIntersectionObservation);
absl::optional<base::TimeTicks> monotonic_time;
UpdateViewportIntersectionsForSubtree(
IntersectionObservation::kFrameViewportIntersectionNeedsUpdate |
IntersectionObservation::kImplicitRootObserversNeedUpdate |
IntersectionObservation::kImplicitRootObserversNeedUpdate |
IntersectionObservation::kIgnoreDelay,
monotonic_time);
}
Expand Down Expand Up @@ -1413,7 +1412,10 @@ void LocalFrameView::ComputePostLayoutIntersections(

if (auto* controller =
GetFrame().GetDocument()->GetIntersectionObserverController()) {
controller->ComputeIntersections(flags, GetUkmAggregator(), monotonic_time);
controller->ComputeIntersections(
flags, GetUkmAggregator(), monotonic_time,
accumulated_scroll_delta_since_last_intersection_update_);
accumulated_scroll_delta_since_last_intersection_update_ = gfx::Vector2dF();
}

for (Frame* child = frame_->Tree().FirstChild(); child;
Expand Down Expand Up @@ -4175,7 +4177,7 @@ void LocalFrameView::CollectAnnotatedRegions(
CollectAnnotatedRegions(*curr, regions);
}

IntersectionUpdateResult LocalFrameView::UpdateViewportIntersectionsForSubtree(
bool LocalFrameView::UpdateViewportIntersectionsForSubtree(
unsigned parent_flags,
absl::optional<base::TimeTicks>& monotonic_time) {
// TODO(dcheng): Since LocalFrameView tree updates are deferred, FrameViews
Expand All @@ -4184,41 +4186,20 @@ IntersectionUpdateResult LocalFrameView::UpdateViewportIntersectionsForSubtree(
// in lifecycle updates are still needed when there are no more deferred
// LocalFrameView updates: https://crbug.com/561683
if (!GetFrame().GetDocument()->IsActive()) {
return IntersectionUpdateResult();
return false;
}

unsigned flags = GetIntersectionObservationFlags(parent_flags);
IntersectionUpdateResult result;
if (RuntimeEnabledFeatures::IntersectionOptimizationEnabled()) {
min_scroll_delta_to_update_intersection_ =
IntersectionGeometry::kInfiniteScrollDelta;
} else {
DCHECK_EQ(min_scroll_delta_to_update_intersection_, gfx::Vector2dF());
}

bool fully_updated = false;
bool needs_occlusion_tracking = false;
if (!NeedsLayout() || IsDisplayLocked()) {
// Notify javascript IntersectionObservers
if (IntersectionObserverController* controller =
GetFrame().GetDocument()->GetIntersectionObserverController()) {
IntersectionUpdateResult observers_result =
controller->ComputeIntersections(flags, GetUkmAggregator(),
monotonic_time);
result.needs_occlusion_tracking =
observers_result.needs_occlusion_tracking;
result.has_implicit_root_observer_with_margin =
observers_result.has_implicit_root_observer_with_margin;
// min_scroll_delta_to_update_intersection_ on this frame is composed
// from the result of updating the IntersectionObservers tracked by this
// frame, and the result of recursive calls to this method for subframes.
// The final value of result.min_scroll_delta_to_update which will be
// returned to the caller is based only on the call to
// UpdateViewportIntersection for this frame.
if (RuntimeEnabledFeatures::IntersectionOptimizationEnabled()) {
min_scroll_delta_to_update_intersection_ =
observers_result.min_scroll_delta_to_update;
}
fully_updated = intersection_observation_state_ >= kDesired;
needs_occlusion_tracking = controller->ComputeIntersections(
flags, GetUkmAggregator(), monotonic_time,
accumulated_scroll_delta_since_last_intersection_update_);
accumulated_scroll_delta_since_last_intersection_update_ =
gfx::Vector2dF();
}
intersection_observation_state_ = kNotNeeded;
}
Expand All @@ -4227,40 +4208,22 @@ IntersectionUpdateResult LocalFrameView::UpdateViewportIntersectionsForSubtree(
SCOPED_UMA_AND_UKM_TIMER(
GetUkmAggregator(),
LocalFrameUkmAggregator::kUpdateViewportIntersection);
// The result is the minimum scroll delta in the parent frame to update
// viewport intersection of this frame.
result.min_scroll_delta_to_update =
UpdateViewportIntersection(flags, result.needs_occlusion_tracking);
}

auto update_result_from_subframe =
[this, &result](const IntersectionUpdateResult& subframe_result) {
result.needs_occlusion_tracking |=
subframe_result.needs_occlusion_tracking;
if (subframe_result.has_implicit_root_observer_with_margin) {
result.has_implicit_root_observer_with_margin = true;
// An implicit-root observer with margin in a subframe requires this
// frame to update intersection observers on every scroll.
min_scroll_delta_to_update_intersection_ = gfx::Vector2dF();
} else {
min_scroll_delta_to_update_intersection_.SetToMin(
subframe_result.min_scroll_delta_to_update);
}
};
UpdateViewportIntersection(flags, needs_occlusion_tracking);
}

for (Frame* child = frame_->Tree().FirstChild(); child;
child = child->Tree().NextSibling()) {
update_result_from_subframe(
needs_occlusion_tracking |=
child->View()->UpdateViewportIntersectionsForSubtree(flags,
monotonic_time));
monotonic_time);
}

if (DocumentPortals* portals = DocumentPortals::Get(*frame_->GetDocument())) {
for (PortalContents* portal : portals->GetPortals()) {
if (Frame* frame = portal->GetFrame()) {
update_result_from_subframe(
needs_occlusion_tracking |=
frame->View()->UpdateViewportIntersectionsForSubtree(
flags, monotonic_time));
flags, monotonic_time);
}
}
}
Expand All @@ -4270,19 +4233,14 @@ IntersectionUpdateResult LocalFrameView::UpdateViewportIntersectionsForSubtree(
for (HTMLFencedFrameElement* fenced_frame :
fenced_frames->GetFencedFrames()) {
if (Frame* frame = fenced_frame->ContentFrame()) {
update_result_from_subframe(
needs_occlusion_tracking |=
frame->View()->UpdateViewportIntersectionsForSubtree(
flags, monotonic_time));
flags, monotonic_time);
}
}
}

if (RuntimeEnabledFeatures::IntersectionOptimizationEnabled() &&
fully_updated) {
accumulated_scroll_delta_since_last_intersection_update_ = gfx::Vector2dF();
}

return result;
return needs_occlusion_tracking;
}

void LocalFrameView::DeliverSynchronousIntersectionObservations() {
Expand Down Expand Up @@ -4412,27 +4370,7 @@ void LocalFrameView::UpdateIntersectionObservationStateOnScroll(
gfx::Vector2dF scroll_delta) {
accumulated_scroll_delta_since_last_intersection_update_ +=
gfx::Vector2dF(std::abs(scroll_delta.x()), std::abs(scroll_delta.y()));
if (intersection_observation_state_ >= kDesired) {
return;
}
if (min_scroll_delta_to_update_intersection_.x() <=
accumulated_scroll_delta_since_last_intersection_update_.x() ||
min_scroll_delta_to_update_intersection_.y() <=
accumulated_scroll_delta_since_last_intersection_update_.y()) {
// The accumulated scroll delta from all scrollers in this frame has
// exceeded min_scroll_delta_to_update_intersection_ since the last
// intersection observer update, which may change intersection status.
SetIntersectionObservationState(kDesired);
} else {
DCHECK(RuntimeEnabledFeatures::IntersectionOptimizationEnabled());
// Frame viewport intersection is always updated on scroll, regardless of
// scroll_delta. Situations such as viewport and main frame intersection
// reporting and implicit-root intersection observers with margins in
// remote frames make the optimization not feasible. Nevertheless, frame
// viewport intersection updates are much faster than intersection
// observer updates, based on UMA data.
SetIntersectionObservationState(kFrameViewportIntersectionOnly);
}
SetIntersectionObservationState(kDesired);
}

void LocalFrameView::InvalidateIntersectionObservations() {
Expand Down Expand Up @@ -4488,18 +4426,14 @@ unsigned LocalFrameView::GetIntersectionObservationFlags(
// Observers with explicit roots only need to be checked on the same frame,
// since in this case target and root must be in the same document.
if (intersection_observation_state_ != kNotNeeded) {
flags |= IntersectionObservation::kFrameViewportIntersectionNeedsUpdate;
}
if (intersection_observation_state_ >= kDesired) {
flags |= (IntersectionObservation::kExplicitRootObserversNeedUpdate |
IntersectionObservation::kImplicitRootObserversNeedUpdate);
}

// For observers with implicit roots, we need to check state on the whole
// local frame tree, as passed down from the parent.
flags |= (parent_flags &
(IntersectionObservation::kFrameViewportIntersectionNeedsUpdate |
IntersectionObservation::kImplicitRootObserversNeedUpdate));
IntersectionObservation::kImplicitRootObserversNeedUpdate);

// The kIgnoreDelay parameter is used to force computation in an OOPIF which
// is hidden in the parent document, thus not running lifecycle updates. It
Expand Down
16 changes: 3 additions & 13 deletions third_party/blink/renderer/core/frame/local_frame_view.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,16 +215,11 @@ class CORE_EXPORT LocalFrameView final
enum IntersectionObservationState {
// The next painting frame does not need an intersection observation.
kNotNeeded = 0,
// The next painting frame only needs to update frame viewport intersection,
// not intersection observations. Note that intersection observations in
// child remote frames happen during the parent frame's viewport
// intersection update, with kDesired or kRequired set on the child frames.
kFrameViewportIntersectionOnly = 1,
// The next painting frame needs an intersection observation.
kDesired = 2,
kDesired = 1,
// The next painting frame must be generated up to intersection observation
// (even if frame is throttled).
kRequired = 3
kRequired = 2
};

// Sets the internal IntersectionObservationState to the max of the
Expand All @@ -243,10 +238,6 @@ class CORE_EXPORT LocalFrameView final

void ForceUpdateViewportIntersections();

gfx::Vector2dF MinScrollDeltaToUpdateIntersectionForTesting() const {
return min_scroll_delta_to_update_intersection_;
}

void SetPaintArtifactCompositorNeedsUpdate();

// Methods for getting/setting the size Blink should use to layout the
Expand Down Expand Up @@ -956,7 +947,7 @@ class CORE_EXPORT LocalFrameView final

void ForAllRemoteFrameViews(base::FunctionRef<void(RemoteFrameView&)>);

IntersectionUpdateResult UpdateViewportIntersectionsForSubtree(
bool UpdateViewportIntersectionsForSubtree(
unsigned parent_flags,
absl::optional<base::TimeTicks>& monotonic_time) override;
void DeliverSynchronousIntersectionObservations();
Expand Down Expand Up @@ -1112,7 +1103,6 @@ class CORE_EXPORT LocalFrameView final
#endif

IntersectionObservationState intersection_observation_state_;
gfx::Vector2dF min_scroll_delta_to_update_intersection_;
gfx::Vector2dF accumulated_scroll_delta_since_last_intersection_update_;

mojom::blink::ViewportIntersectionState last_intersection_state_;
Expand Down
Loading

0 comments on commit 339bcd9

Please sign in to comment.