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

Issue 635015 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 634696



Sign in to add a comment

Avoid allocation in GrBufferAllocPool::reset()

Project Member Reported by dskiba@chromium.org, Aug 5 2016

Issue description

As part of identifying redundant allocations in Chrome, I found that GrBufferAllocPool::resetCpuData() sometimes allocates 128 KiB of zero buffers (4 * 32 KiB): https://docs.google.com/spreadsheets/d/1HfHKXy4q-ytZ-nwPay8YYyVSn1o-X9I7cWzYlygvtwQ/edit#gid=810318063

It's not very clear from the stacktrace (because of the inlining), but in that case call chain looked like this: GrDrawingManager::flush() -> GrDrawingManager::reset() -> GrBufferAllocPool::reset() -> GrBufferAllocPool::resetCpuData().

It seems that reset() can just call resetCpuData(0) instead of resetCpuData(fMinBlockSize) to avoid allocating zero memory:

* resetCpuData(0) will free and null fCpuData
* fCpuData is not directly used anywhere outside of resetCpuData(), so no one cares if it's NULL
* The only other place (createBlock) that calls resetCpuData() won't ever notice the difference, because resetCpuData() always reallocates


(More info about the effort to kill redundant allocations is here: https://groups.google.com/a/chromium.org/forum/#!msg/project-trim/EiKXwnbTvbo/0z-Y5Kg1CgAJ)

 
Blocking: 634696

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

Cc: tomhud...@chromium.org
Please take a look.
Owner: robertphillips@chromium.org
Rob's more familiar with the context, or can delegate to the right person on the Ganesh team.
Project Member

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

The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/48d91b52e4eb27f6bcb8eadb68558707a3c30875

commit 48d91b52e4eb27f6bcb8eadb68558707a3c30875
Author: robertphillips <robertphillips@google.com>
Date: Thu Aug 18 21:01:14 2016

Delay creation of cpu-side buffer memory until actually needed

IIUC what is going on, this won't really do anything bad but will defer allocation of the cpu-side buffer until it is actually needed.

BUG= 635015 
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2248283007

Review-Url: https://codereview.chromium.org/2248283007

[modify] https://crrev.com/48d91b52e4eb27f6bcb8eadb68558707a3c30875/src/gpu/GrBufferAllocPool.cpp

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 18 2016

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

commit 4a37f15477179759c467b9adf91bb07d54ec230e
Author: skia-deps-roller <skia-deps-roller@chromium.org>
Date: Thu Aug 18 22:43:17 2016

Roll src/third_party/skia/ 3896effca..530032a18 (5 commits).

https://chromium.googlesource.com/skia.git/+log/3896effcad35..530032a18e37

$ git log 3896effca..530032a18 --date=short --no-merges --format='%ad %ae %s'
2016-08-18 halcanary SkPDF: in-place font subsetting
2016-08-18 msarett Fix initialization bug for fConservativeIsScaleTranslate
2016-08-18 robertphillips Delay creation of cpu-side buffer memory until actually needed
2016-08-18 msarett Batched implementation of drawLattice() for GPU
2016-08-18 reed make LayerIter private, and remove skipClip option

BUG= 635015 

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
TBR=robertphillips@google.com

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

[modify] https://crrev.com/4a37f15477179759c467b9adf91bb07d54ec230e/DEPS

Status: Fixed (was: Untriaged)

Sign in to add a comment