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

Issue 638064 link

Starred by 1 user

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: vcall -> constant for all but one, and direct call for one type

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

Issue description

approximateOpCount is called from DrawingDisplayItem::ApproximateOpCount and shows a visible overhead due to slow virtual calls:
https://cs.chromium.org/chromium/src/third_party/skia/include/core/SkPicture.h?sq=package:chromium&dr=CSs&rcl=1471276009&l=145

The implementations:

SkEmptyPicture::approximateOpCount() { return 0; }
SkMiniPicture::approximateOpCount() { return 1; }
SkBigPicture::approximateOpCount() { return fRecord->count(); }

and the last one is actually equivalent to return fRecord->fCount.

This can be turned into a simple type switch, similar to https://crbug.com/638060
 

Comment 1 by krasin@chromium.org, Aug 18 2016

Summary: devirt-want: SkPicture::approximateOpCount and approximateBytesUsed (was: devirt-want: SkPicture::approximateOpCount)
Another similar case is SkPicture::approximateBytesUsed. It has three implementations:

size_t SkEmptyPicture::approximateBytesUsed() const override { return sizeof(*this); }
size_t SkMiniPicture::approximateBytesUsed() const override { return sizeof(*this); }

size_t SkBigPicture::approximateBytesUsed() const {
    size_t bytes = sizeof(*this) + fRecord->bytesUsed() + fApproxBytesUsedBySubPictures;
    if (fBBH) { bytes += fBBH->bytesUsed(); }
    return bytes;
}

This can be devirtualized to:

if vptr == SkBigPicture {
  direct call SkBigPicture::approximateBytesUsed
} else {
  read virtual const from vtable // sizeof(*this)
}

Comment 2 by krasin@chromium.org, Aug 18 2016

One more case: SkBitmapDevice::onAccessPixels:
https://cs.chromium.org/chromium/src/third_party/skia/include/core/SkDevice.h?sq=package:chromium&dr=CSs&rcl=1471534131&l=292

Implementations:

bool SkDevice::onAccessPixels(SkPixmap*) { return false; }

bool SkGpuDevice::onAccessPixels(SkPixmap* pmap) {
    ASSERT_SINGLE_OWNER // debug only
    return false;
}

bool SkBitmapDevice::onAccessPixels(SkPixmap* pmap) {
    if (this->onPeekPixels(pmap)) {
        fBitmap.notifyPixelsChanged();
        return true;
    }
    return false;
}

Once again, it's simple rewrite:

if vptr == SkBitmapDevice {
  direct call SkBitmapDevice::onAccessPixels
} else {
  constant false
}

Comment 3 by krasin@chromium.org, Aug 18 2016

Summary: devirt-want: vcall -> constant for all but one, and direct call to cover the rest (was: devirt-want: SkPicture::approximateOpCount and approximateBytesUsed)

Comment 4 by krasin@chromium.org, Aug 18 2016

Summary: devirt-want: vcall -> constant for all but one, and direct call for one type (was: devirt-want: vcall -> constant for all but one, and direct call to cover the rest)
Status: Assigned (was: Untriaged)

Sign in to add a comment