Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2201)

Issue 6199075: add protected setPreLocked, for pixelrefs who never need the onLockPixels (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 5 months ago by reed1
Modified:
12 years, 5 months ago
Reviewers:
Leon, DerekS
CC:
skia-review_googlegroups.com
Base URL:
http://skia.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Mutexes in pixelrefs were done very sloppily initially. The code (a) assumes all pixelref subclasses want a mutex to guard their lock/unlock virtuals, and (b) most subclasses use the same mutex for *all* of their instances, even when there is no explicit need to guard modifying one instances with another. When we try drawing bitmaps from multiple threads, we are seeing a lot of slow- down from these mutexes. This CL has two changes to try to speed things up. 1. Add setPreLocked(), for pixelrefs who never need the onLockPixels virtual to be called. This speeds up those subclasses in multithreaded environs as it avoids the mutex lock all together (e.g. SkMallocPixelRef). 2. Add setMutex() to allow a subclass to change the mutex choice. ashmem wants this, since its unflattening constructor cannot pass down the null, it needs to cleanup afterwards. Committed: https://code.google.com/p/skia/source/detail?r=3985

Patch Set 1 #

Patch Set 2 : #

Total comments: 1

Patch Set 3 : #

Patch Set 4 : #

Patch Set 5 : #

Patch Set 6 : #

Patch Set 7 : #

Patch Set 8 : #

Total comments: 1

Patch Set 9 : #

Patch Set 10 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+100 lines, -28 lines) Patch
M include/core/SkPixelRef.h View 1 2 3 4 5 6 7 8 9 3 chunks +20 lines, -3 lines 0 comments Download
M include/core/SkThread_platform.h View 1 2 3 4 5 6 7 8 9 2 chunks +5 lines, -2 lines 0 comments Download
M src/core/SkBitmap.cpp View 1 2 3 4 5 6 7 8 9 2 chunks +2 lines, -2 lines 0 comments Download
M src/core/SkBitmapProcShader.cpp View 1 2 3 4 5 6 7 8 9 2 chunks +2 lines, -2 lines 0 comments Download
M src/core/SkMallocPixelRef.cpp View 1 2 3 4 5 6 7 8 9 2 chunks +4 lines, -0 lines 0 comments Download
M src/core/SkPixelRef.cpp View 1 2 3 4 5 6 7 8 9 2 chunks +61 lines, -16 lines 0 comments Download
M src/images/SkImageRefPool.cpp View 1 2 3 4 5 6 7 8 9 2 chunks +3 lines, -3 lines 0 comments Download
M src/ports/SkImageRef_ashmem.cpp View 1 2 3 4 5 6 7 8 9 2 chunks +3 lines, -0 lines 0 comments Download

Messages

Total messages: 6
reed1
1. Does this work for you in your multi-threaded case? 2. Is it measurably faster?
12 years, 5 months ago (2012-05-14 18:05:03 UTC) #1
reed1
1.1 Does this work correctly for you in a debug-build with asserts?
12 years, 5 months ago (2012-05-14 18:05:46 UTC) #2
Leon
https://codereview.appspot.com/6199075/diff/2001/src/core/SkMallocPixelRef.cpp File src/core/SkMallocPixelRef.cpp (right): https://codereview.appspot.com/6199075/diff/2001/src/core/SkMallocPixelRef.cpp#newcode61 src/core/SkMallocPixelRef.cpp:61: } Needs this->setPreLocked(fStorage, fCTable); Otherwise we lose the prelocked ...
12 years, 5 months ago (2012-05-14 18:47:27 UTC) #3
reed1
change getLockCount() to isLocked(), since the magnitude of the lock-count is now funny, given the ...
12 years, 5 months ago (2012-05-14 19:39:36 UTC) #4
reed1
1. renamed public method for ashmem to useDefaultMutex() 2. s/matrix/mutex
12 years, 5 months ago (2012-05-14 19:51:09 UTC) #5
Leon
12 years, 5 months ago (2012-05-15 13:08:31 UTC) #6
https://codereview.appspot.com/6199075/diff/14001/include/core/SkPixelRef.h
File include/core/SkPixelRef.h (right):

https://codereview.appspot.com/6199075/diff/14001/include/core/SkPixelRef.h#n...
include/core/SkPixelRef.h:178: // only call from constructor. Specify a
(possibly) different mutex, or
This comment describes setMutex (below), rather than useDefaultMutex
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b