Instance counting for SkRefCnt-derived objects

What do you all think about this as a debugging aide? 

Right now (on a trimmed down GM) 22 SkRefCnt-derived objects are being reported at the end of gmmain. After the assert only the GrGLInterface stored in GrGLCreateDebugInterface is being deleted -> 21 objects MIA.

[bsalomon, 2012-05-30 19:32:22]

On 2012/05/30 19:07:55, robertphillips wrote:

In general I'm interested in this type of debugging facility. I think it might
take a bit more to make it truly useful. Should we have a short meeting to throw
out some ideas?

[robertphillips, 2012-05-30 19:52:08]

Sure - I'm ready whenever you all are.

[robertphillips, 2012-05-30 21:44:56]

In addition to being macro-based, this CL also updates GrResource to illustrate
how invasive this is going to be.

[bsalomon, 2012-05-31 12:39:53]

On 2012/05/30 21:44:56, robertphillips wrote:
> In addition to being macro-based, this CL also updates GrResource to
illustrate
> how invasive this is going to be.

LGTM, modulo nits.

[bsalomon, 2012-05-31 12:40:01]

http://codereview.appspot.com/6242070/diff/6/include/core/SkRefCnt.h
File include/core/SkRefCnt.h (right):

http://codereview.appspot.com/6242070/diff/6/include/core/SkRefCnt.h#newcode30
include/core/SkRefCnt.h:30: static int32_t getInstanceCount() { \
cap for static (GetInstanceCount())

http://codereview.appspot.com/6242070/diff/6/include/core/SkRefCnt.h#newcode34
include/core/SkRefCnt.h:34: static int32_t  fInstanceCount;
gInstanceCount

http://codereview.appspot.com/6242070/diff/6/src/core/SkRefCnt.cpp
File src/core/SkRefCnt.cpp (right):

http://codereview.appspot.com/6242070/diff/6/src/core/SkRefCnt.cpp#newcode13
src/core/SkRefCnt.cpp:13: 
Are we going to get empty translation unit warnings in a release build? (do we
care?)

[bungeman, 2012-05-31 14:02:31]

This may be more of a lament than a suggestion, but SkPostConfig.h has SkNEW and
related defines. If we used these in general it would be much easier to have
versions of them which were friendly to memory checkers (as the file and line
number of the caller could be passed to 'new'). It would then be much easier to
do simpler and more general memory fuzzing.

On 2012/05/31 12:40:01, bsalomon wrote:
> http://codereview.appspot.com/6242070/diff/6/include/core/SkRefCnt.h
> File include/core/SkRefCnt.h (right):
> 
> http://codereview.appspot.com/6242070/diff/6/include/core/SkRefCnt.h#newcode30
> include/core/SkRefCnt.h:30: static int32_t getInstanceCount() { \
> cap for static (GetInstanceCount())
> 
> http://codereview.appspot.com/6242070/diff/6/include/core/SkRefCnt.h#newcode34
> include/core/SkRefCnt.h:34: static int32_t  fInstanceCount;
> gInstanceCount
> 
> http://codereview.appspot.com/6242070/diff/6/src/core/SkRefCnt.cpp
> File src/core/SkRefCnt.cpp (right):
> 
> http://codereview.appspot.com/6242070/diff/6/src/core/SkRefCnt.cpp#newcode13
> src/core/SkRefCnt.cpp:13: 
> Are we going to get empty translation unit warnings in a release build? (do we
> care?)

[robertphillips, 2012-05-31 14:43:51]

Not super cool but hopefully better.

http://codereview.appspot.com/6242070/diff/6/include/core/SkRefCnt.h
File include/core/SkRefCnt.h (right):

http://codereview.appspot.com/6242070/diff/6/include/core/SkRefCnt.h#newcode30
include/core/SkRefCnt.h:30: static int32_t getInstanceCount() { \
On 2012/05/31 12:40:01, bsalomon wrote:
> cap for static (GetInstanceCount())

Done.

http://codereview.appspot.com/6242070/diff/6/include/core/SkRefCnt.h#newcode34
include/core/SkRefCnt.h:34: static int32_t  fInstanceCount;
On 2012/05/31 12:40:01, bsalomon wrote:
> gInstanceCount

Done.

http://codereview.appspot.com/6242070/diff/6/src/core/SkRefCnt.cpp
File src/core/SkRefCnt.cpp (right):

http://codereview.appspot.com/6242070/diff/6/src/core/SkRefCnt.cpp#newcode13
src/core/SkRefCnt.cpp:13: 
Yes we will - the only solution I can think of is to put the definition in
another file we know will always be around. SkStream.cpp?

[reed1, 2012-05-31 14:46:43]

much nicer.

can we move the macro definitions into their own header? Doesn't seem like this
is specific to refcounting any more!

[bsalomon, 2012-05-31 14:58:57]

Definitely better.

[robertphillips, 2012-05-31 15:20:16]

Moved macros. Brian - any feelings on hiding SkRefCnt's definition in another
file?

[bsalomon, 2012-05-31 15:26:49]

On 2012/05/31 15:20:16, robertphillips wrote:
> Moved macros. Brian - any feelings on hiding SkRefCnt's definition in another
> file?

I took a look at the files in core and none seemed like an obvious home to me.
I'm OK with the new cpp.

[reed1, 2012-05-31 17:10:46]

new files should get the new copyright 

/*
 * Copyright 2012 Google Inc.
 *
 * Use of this source code is governed by a BSD-style license that can be
 * found in the LICENSE file.
 */

[robertphillips, 2012-06-04 12:40:04]

On 2012/05/31 17:10:46, reed1 wrote:
> new files should get the new copyright 
> 
> /*
>  * Copyright 2012 Google Inc.
>  *
>  * Use of this source code is governed by a BSD-style license that can be
>  * found in the LICENSE file.
>  */

Done.

[reed1, 2012-06-04 12:42:30]

lgtm

lets prepare the DEPS roll w/ associated skia.gyp change right after this lands.

[robertphillips, 2012-06-05 15:41:26]

committed as r4162

[robertphillips, 2012-06-05 19:27:15]

rolled back in r4164

[robertphillips, 2012-06-05 19:50:53]

received verbal LGTM on CanvasTest changes from Brian

committed as r4170