New issue
Advanced search Search tips

Issue 670926 link

Starred by 1 user
Status: Fixed
Owner:
khushals...@chromium.org
Closed: Dec 2016
Cc:
vmp...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Regression

Blocked on:
issue 665266


Sign in to add a comment

Computing ImageToRegionMap is a large perf regression on MotionMark Leaves test

Project Member Reported by vmi...@chromium.org, Dec 3 2016 
Chrome Version: r436112
OS: All

Run the Animometer > Leaves test at http://browserbench.org/MotionMark/developer.html.

On my MBP at Leaves complexity 1000, there's a 2.5ms / frame increase in RecordingSource::FinishDisplayItemListUpdate times.

I think this needs to move off the critical path.  Since we don't need to look up image invalidation regions frequently, perhaps GetRegionForImage() could compute them on-demand from the image_set_?

Comment 1 by vmi...@chromium.org, Dec 3 2016

Labels: ReleaseBlock-Stable M-56
I'm going to revert https://chromium.googlesource.com/chromium/src/+/fc85c1aa0881047c4410cbedc1e4543bcf789a1f for now.

This also is on the M56 branch.
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 3 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/654417a5699ed0965a94977350266bf5ce206daa

commit 654417a5699ed0965a94977350266bf5ce206daa
Author: vmiura <vmiura@chromium.org>
Date: Sat Dec 03 21:10:01 2016

Revert of cc: Build Image to Region map during image meta-data generation. (patchset #6 id:100001 of https://codereview.chromium.org/2496283004/ )

Reason for revert:
Computing these image regions adds significant time to the critical path on image heavy content.  Let's re-land after addressing the performance.

BUG= 670926 

Original issue's description:
> cc: Build Image to Region map during image meta-data generation.
>
> During the discardable image meta-data generation phase, we build an
> Rtree to pull the set of images given a rect. This change adds a
> reverse lookup map to be built at the same time for querying the region
> for an image in a DisplayItemList.
>
> BUG= 665266 
> CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
>
> Committed: https://crrev.com/fc85c1aa0881047c4410cbedc1e4543bcf789a1f
> Cr-Commit-Position: refs/heads/master@{#432841}

TBR=vmpstr@chromium.org,khushalsagar@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 665266 

Review-Url: https://codereview.chromium.org/2551583003
Cr-Commit-Position: refs/heads/master@{#436174}

[modify] https://crrev.com/654417a5699ed0965a94977350266bf5ce206daa/cc/playback/discardable_image_map.cc
[modify] https://crrev.com/654417a5699ed0965a94977350266bf5ce206daa/cc/playback/discardable_image_map.h
[modify] https://crrev.com/654417a5699ed0965a94977350266bf5ce206daa/cc/playback/discardable_image_map_unittest.cc
[modify] https://crrev.com/654417a5699ed0965a94977350266bf5ce206daa/cc/playback/display_item_list.cc
[modify] https://crrev.com/654417a5699ed0965a94977350266bf5ce206daa/cc/playback/display_item_list.h

Comment 3 by vmi...@chromium.org, Dec 3 2016

Labels: Merge-Request-56

Comment 4 by dimu@chromium.org, Dec 4 2016

Labels: -Merge-Request-56 Merge-Approved-56 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M56 (branch: 2924)

Comment 5 by ligim...@chromium.org, Dec 5 2016 

M56 Beta promotion is scheduled on Dec 7 .Please ensure to verify the fix and merge your change ASAP so that we could take it for next Release.
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 6 2016

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c006456e1c0057757977351133fd7bb2e5275b90

commit c006456e1c0057757977351133fd7bb2e5275b90
Author: Victor Miura <vmiura@chromium.org>
Date: Tue Dec 06 00:06:03 2016

Revert of cc: Build Image to Region map during image meta-data generation. (patchset #6 id:100001 of https://codereview.chromium.org/2496283004/ )

Reason for revert:
Computing these image regions adds significant time to the critical path on image heavy content.  Let's re-land after addressing the performance.

BUG= 670926 

Original issue's description:
> cc: Build Image to Region map during image meta-data generation.
>
> During the discardable image meta-data generation phase, we build an
> Rtree to pull the set of images given a rect. This change adds a
> reverse lookup map to be built at the same time for querying the region
> for an image in a DisplayItemList.
>
> BUG= 665266 
> CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
>
> Committed: https://crrev.com/fc85c1aa0881047c4410cbedc1e4543bcf789a1f
> Cr-Commit-Position: refs/heads/master@{#432841}

TBR=vmpstr@chromium.org,khushalsagar@chromium.org
BUG= 665266 

Review-Url: https://codereview.chromium.org/2551583003
Cr-Commit-Position: refs/heads/master@{#436174}
(cherry picked from commit 654417a5699ed0965a94977350266bf5ce206daa)

Review URL: https://codereview.chromium.org/2552903002 .

Cr-Commit-Position: refs/branch-heads/2924@{#347}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/c006456e1c0057757977351133fd7bb2e5275b90/cc/playback/discardable_image_map.cc
[modify] https://crrev.com/c006456e1c0057757977351133fd7bb2e5275b90/cc/playback/discardable_image_map.h
[modify] https://crrev.com/c006456e1c0057757977351133fd7bb2e5275b90/cc/playback/discardable_image_map_unittest.cc
[modify] https://crrev.com/c006456e1c0057757977351133fd7bb2e5275b90/cc/playback/display_item_list.cc
[modify] https://crrev.com/c006456e1c0057757977351133fd7bb2e5275b90/cc/playback/display_item_list.h

Comment 7 by vmi...@chromium.org, Dec 6 2016

Blockedon: 665266
Status: Fixed (was: Assigned)
Closing as Fixed.   Issue 665266  is open for follow-up work getting this feature re-landed.
►

Sign in to add a comment