New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 638045 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Last visit > 30 days ago
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

devirt-want: Layer::SetNeedsDisplayRect

Project Member Reported by krasin@chromium.org, Aug 16 2016

Issue description

Layer::SetNeedsDisplayRect is a virtual method called from  WebLayerImpl::invalidateRect:
https://cs.chromium.org/chromium/src/cc/blink/web_layer_impl.cc?q=WebLayerImpl::invalidateRect&sq=package:chromium&l=59&dr=CSs

This proved to have a negative perf impact under CFI, which means the virtual call overhead is high relative to the actual work being done. See  https://crbug.com/634139  for more details.

It would be nice to automatically devirtualize the calls to the method. Let's analyze its implementations:

1. Layer::SetNeedsDisplayRect is quite complicated:
https://cs.chromium.org/chromium/src/cc/layers/layer.cc?q=SetNeedsDisplayRect&sq=package:chromium&l=1063&dr=CSs

2. PictureLayer::SetNeedsDisplayRect is just:
https://cs.chromium.org/chromium/src/cc/layers/picture_layer.cc?sq=package:chromium&dr=CSs&rcl=1471276009&l=88

if (recording_source_)
    recording_source_->SetNeedsDisplayRect(layer_rect);
Layer::SetNeedsDisplayRect(layer_rect);

3. SolidColorScrollbarLayer::SetNeedsDisplayRect is empty:
https://cs.chromium.org/chromium/src/cc/layers/solid_color_scrollbar_layer.cc?sq=package:chromium&dr=CSs&rcl=1471276009&l=88

4. TextureLayer::SetNeedsDisplayRect can be deleted right away as it just calls Layer::SetNeedsDisplayRect (and Layer is its immediate base class):
https://cs.chromium.org/chromium/src/cc/layers/texture_layer.cc?sq=package:chromium&dr=CSs&rcl=1471276009&l=157
void TextureLayer::SetNeedsDisplayRect(const gfx::Rect& dirty_rect) {
  Layer::SetNeedsDisplayRect(dirty_rect);
}

The devirtualized call site would look the following (pseudo-code):
if (type != SolidColorScrollbarLayer) {
  if ((type == PictureLayer) && recording_source_)
    recording_source_->SetNeedsDisplayRect(layer_rect);
  Layer::SetNeedsDisplayRect(rect);
}

where RecordingSource::SetNeedsDisplayRect is non-virtual:
https://cs.chromium.org/chromium/src/cc/playback/recording_source.h?sq=package:chromium&dr=CSs&rcl=1471276009&l=66

This should be faster than a virtual call. The only problem I see is that we need more cases like this to justify implementing the optimization. There very well could be.
 
Status: Assigned (was: Untriaged)

Sign in to add a comment