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

Issue 5649046: WeakRefCnt (Closed)

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

Patch Set 1 #

Total comments: 3

Patch Set 2 : Add SkVirtualRefCnt, make SkTypeface one. #

Patch Set 3 : Make SkRefCnt weakly usable. #

Patch Set 4 : Add some implementations of sk_atomic_conditional_inc. #

Patch Set 5 : Cleanup and additional sk_atomic_conditional_inc. #

Patch Set 6 : Now with memory barriers and explanations. #

Total comments: 4

Patch Set 7 : Remove extra '{'; warn when ignoring result of weak_lock. #

Patch Set 8 : Split out new function into SkWeakRef. #

Total comments: 8

Patch Set 9 : Add clarifying comment, line lengths. #

Total comments: 5

Patch Set 10 : Add a bench. #

Patch Set 11 : Something which might be checked in. #

Total comments: 2

Patch Set 12 : Lipstick applied. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+529 lines, -41 lines) Patch
M bench/RefCntBench.cpp View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +119 lines, -4 lines 0 comments Download
M gyp/core.gyp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -0 lines 0 comments Download
M gyp/ports.gyp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +6 lines, -2 lines 0 comments Download
M include/core/SkRefCnt.h View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +26 lines, -13 lines 0 comments Download
M include/core/SkThread.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -0 lines 0 comments Download
M include/core/SkThread_platform.h View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +67 lines, -2 lines 0 comments Download
M include/core/SkTypeface.h View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +3 lines, -3 lines 0 comments Download
A include/core/SkWeakRefCnt.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +155 lines, -0 lines 0 comments Download
M src/core/SkTypefaceCache.h View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +7 lines, -4 lines 0 comments Download
M src/core/SkTypefaceCache.cpp View 1 2 3 4 5 6 7 8 9 10 11 7 chunks +35 lines, -12 lines 0 comments Download
M src/ports/SkThread_none.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +8 lines, -0 lines 0 comments Download
M src/ports/SkThread_pthread.cpp View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +32 lines, -0 lines 0 comments Download
M src/ports/SkThread_win.cpp View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +17 lines, -0 lines 0 comments Download
M tests/RefCntTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +52 lines, -1 line 0 comments Download

Messages

Total messages: 22
bungeman
This change introduces SkWeakRefCnt and SkWeakRefCntable and makes SkTypeface SkWeakRefCntable. The changes to SkFontHost_win and ...
12 years, 10 months ago (2012-02-09 22:08:18 UTC) #1
TomH
Why do we need this? http://codereview.appspot.com/5649046/diff/1/src/core/SkWeakRefCnt.h File src/core/SkWeakRefCnt.h (right): http://codereview.appspot.com/5649046/diff/1/src/core/SkWeakRefCnt.h#newcode13 src/core/SkWeakRefCnt.h:13: A strong SkWeakRefCnt::expired() will ...
12 years, 10 months ago (2012-02-09 22:10:18 UTC) #2
bungeman
The reason we want this is for typefaces created from streams. We need to be ...
12 years, 10 months ago (2012-02-09 22:31:10 UTC) #3
bungeman
A different approach will need to be taken. http://codereview.appspot.com/5649046/diff/1/src/ports/SkFontHost_win.cpp File src/ports/SkFontHost_win.cpp (right): http://codereview.appspot.com/5649046/diff/1/src/ports/SkFontHost_win.cpp#newcode171 src/ports/SkFontHost_win.cpp:171: SkSafeRef(fTarget); ...
12 years, 10 months ago (2012-02-10 18:57:07 UTC) #4
bungeman
This makes the ref and unref on SkTypeface virtual. Once again the gyp and font ...
12 years, 10 months ago (2012-02-16 20:36:16 UTC) #5
bungeman
Patch set 3 is a draft of how to allow SkRefCnt objects to be weakly ...
12 years, 10 months ago (2012-02-17 18:26:21 UTC) #6
bungeman
After much todo, this is now ready for review. As mentioned before, this CL contains ...
12 years, 8 months ago (2012-04-11 22:27:38 UTC) #7
Steve VanDeBogart
A couple drive-by comments. http://codereview.appspot.com/5649046/diff/13001/include/core/SkRefCnt.h File include/core/SkRefCnt.h (right): http://codereview.appspot.com/5649046/diff/13001/include/core/SkRefCnt.h#newcode99 include/core/SkRefCnt.h:99: bool weak_lock() const { I ...
12 years, 8 months ago (2012-04-11 22:48:54 UTC) #8
bungeman
http://codereview.appspot.com/5649046/diff/13001/include/core/SkRefCnt.h File include/core/SkRefCnt.h (right): http://codereview.appspot.com/5649046/diff/13001/include/core/SkRefCnt.h#newcode99 include/core/SkRefCnt.h:99: bool weak_lock() const { On 2012/04/11 22:48:54, Steve VanDeBogart ...
12 years, 8 months ago (2012-04-11 23:18:45 UTC) #9
bungeman
This splits SkWeakRefCnt out into its own class which inherits from SkRefCnt. This reduces the ...
12 years, 7 months ago (2012-05-07 22:33:46 UTC) #10
Steve VanDeBogart
http://codereview.appspot.com/5649046/diff/23001/include/core/SkRefCnt.h File include/core/SkRefCnt.h (right): http://codereview.appspot.com/5649046/diff/23001/include/core/SkRefCnt.h#newcode111 include/core/SkRefCnt.h:111: /** Default construct, initializing the reference counts to 1. ...
12 years, 7 months ago (2012-05-08 00:21:46 UTC) #11
bungeman
http://codereview.appspot.com/5649046/diff/23001/include/core/SkRefCnt.h File include/core/SkRefCnt.h (right): http://codereview.appspot.com/5649046/diff/23001/include/core/SkRefCnt.h#newcode111 include/core/SkRefCnt.h:111: /** Default construct, initializing the reference counts to 1. ...
12 years, 7 months ago (2012-05-08 13:51:52 UTC) #12
Steve VanDeBogart
http://codereview.appspot.com/5649046/diff/23001/include/core/SkRefCnt.h File include/core/SkRefCnt.h (right): http://codereview.appspot.com/5649046/diff/23001/include/core/SkRefCnt.h#newcode111 include/core/SkRefCnt.h:111: /** Default construct, initializing the reference counts to 1. ...
12 years, 7 months ago (2012-05-08 22:52:45 UTC) #13
bungeman
I'll fix up the line lengths before checking anything in. http://codereview.appspot.com/5649046/diff/23001/include/core/SkRefCnt.h File include/core/SkRefCnt.h (right): http://codereview.appspot.com/5649046/diff/23001/include/core/SkRefCnt.h#newcode111 ...
12 years, 7 months ago (2012-05-09 18:56:19 UTC) #14
Steve VanDeBogart
On 2012/05/09 18:56:19, bungeman wrote: > I'll fix up the line lengths before checking anything ...
12 years, 7 months ago (2012-05-10 00:05:16 UTC) #15
bungeman
This patch set (11) contains something which could be checked in. Running the bench included ...
12 years, 7 months ago (2012-05-14 19:40:14 UTC) #16
bungeman
On 2012/05/14 19:40:14, bungeman wrote: > This patch set (11) contains something which could be ...
12 years, 7 months ago (2012-05-14 19:50:08 UTC) #17
reed1
lgtm
12 years, 7 months ago (2012-05-14 19:52:48 UTC) #18
reed1
http://codereview.appspot.com/5649046/diff/27018/include/core/SkRefCnt.h File include/core/SkRefCnt.h (right): http://codereview.appspot.com/5649046/diff/27018/include/core/SkRefCnt.h#newcode141 include/core/SkRefCnt.h:141: */ other names? - attempt_ref() - strong_ref() - try_foo_ref() ...
12 years, 7 months ago (2012-05-14 20:03:13 UTC) #19
reed1
can we move this to its own header?
12 years, 7 months ago (2012-05-14 20:03:40 UTC) #20
bungeman
Updated documentation and changed weak_lock to try_ref. http://codereview.appspot.com/5649046/diff/27018/include/core/SkRefCnt.h File include/core/SkRefCnt.h (right): http://codereview.appspot.com/5649046/diff/27018/include/core/SkRefCnt.h#newcode141 include/core/SkRefCnt.h:141: */ On ...
12 years, 7 months ago (2012-05-14 21:06:54 UTC) #21
bungeman
12 years, 7 months ago (2012-05-17 18:56:10 UTC) #22
Committed revision r3978.
Android issues committed revision r3979 and r3982.
Sign in to reply to this message.

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