Split atomic and mutex implementations and make inlinable.

Split atomic and mutex implementations and make inlinable.

Skia cannot use Chromium's implementation of mutex (Lock) due to static
initializers. However, we would like to be able to use Chromium's
implementation of atomics. This motivates the split of implementation.

Skia's atomic and mutex calls should be inlinable, especially the atomics.
These calls often compile down to very few instructions, and we currently have
the overhead of a function call. This motivates the header implementation.

There is still a desire for the build system to select the implementation, so
the SK_XXX_PLATFORM_H pattern for header files is introduced. This allows the
build system to control which platform specific header files are chosen.

The Chromium side changes (most of which will need to go in before this change
can be found at https://codereview.chromium.org/19477005/ .
The Chromium side changes after this lands can be seen at 
https://codereview.chromium.org/98073013 .

Committed: https://code.google.com/p/skia/source/detail?r=12738

[djsollen, 2013-07-25 16:00:34]

https://codereview.chromium.org/19808007/diff/20001/gyp/core.gyp
File gyp/core.gyp (right):

https://codereview.chromium.org/19808007/diff/20001/gyp/core.gyp#newcode16
gyp/core.gyp:16: }, {
Is this where we are supposed to put the android framework atomic check?  That
is when I add the skia_android_framework variable to gyp.

https://codereview.chromium.org/19808007/diff/20001/include/core/SkThread.h
File include/core/SkThread.h (right):

https://codereview.chromium.org/19808007/diff/20001/include/core/SkThread.h#n...
include/core/SkThread.h:20: /** Atomically adds inc to the int referenced by
addrand returns the previous value.
missing space in 'addrand'

https://codereview.chromium.org/19808007/diff/20001/include/core/SkThread.h#n...
include/core/SkThread.h:39: static void sk_membar_acquire__after_atomic_dec();
do we need these two membar_aquire* functions as it appears all clients do
nothing.  What value do they add?

https://codereview.chromium.org/19808007/diff/20001/include/core/SkThread.h#n...
include/core/SkThread.h:49: /** SK_MUTEX_PLATFORM_H must provide the following
(or equivelent) declarations.
equivalent

https://codereview.chromium.org/19808007/diff/20001/src/ports/SkMutex_none.h
File src/ports/SkMutex_none.h (right):

https://codereview.chromium.org/19808007/diff/20001/src/ports/SkMutex_none.h#...
src/ports/SkMutex_none.h:25: #define SK_DECLARE_STATIC_MUTEX(name) static
SkBaseMutex name = { }
do you need the '{ }' here?

[djsollen, 2013-07-25 18:17:21]

https://codereview.chromium.org/19808007/diff/20001/src/ports/SkAtomics_andro...
File src/ports/SkAtomics_android.h (right):

https://codereview.chromium.org/19808007/diff/20001/src/ports/SkAtomics_andro...
src/ports/SkAtomics_android.h:15: #include <utils/Atomic.h>
we should update this to

#include <cutils/atomic.h>

[bungeman-skia, 2013-09-26 17:16:23]

https://codereview.chromium.org/19808007/diff/20001/gyp/core.gyp
File gyp/core.gyp (right):

https://codereview.chromium.org/19808007/diff/20001/gyp/core.gyp#newcode16
gyp/core.gyp:16: }, {
On 2013/07/25 16:00:34, djsollen wrote:
> Is this where we are supposed to put the android framework atomic check?  That
> is when I add the skia_android_framework variable to gyp.

Yes, when we get a skia_android_framework I suppose that it would point atomics
at _android and mutexes at _pthread.

https://codereview.chromium.org/19808007/diff/20001/include/core/SkThread.h
File include/core/SkThread.h (right):

https://codereview.chromium.org/19808007/diff/20001/include/core/SkThread.h#n...
include/core/SkThread.h:20: /** Atomically adds inc to the int referenced by
addrand returns the previous value.
On 2013/07/25 16:00:34, djsollen wrote:
> missing space in 'addrand'

Done.

https://codereview.chromium.org/19808007/diff/20001/include/core/SkThread.h#n...
include/core/SkThread.h:39: static void sk_membar_acquire__after_atomic_dec();
On 2013/07/25 16:00:34, djsollen wrote:
> do we need these two membar_aquire* functions as it appears all clients do
> nothing.  What value do they add?

Correctness and documentation. Many (all?) lock-free reference counting
implementations I've seen completely fail to properly document which barriers
are needed where. Also, these should be implemented on ARM. Our Android
framework port is currently relying on the Android atomics being full memory
barriers, but we really shouldn't be depending on that.

https://codereview.chromium.org/19808007/diff/20001/include/core/SkThread.h#n...
include/core/SkThread.h:49: /** SK_MUTEX_PLATFORM_H must provide the following
(or equivelent) declarations.
On 2013/07/25 16:00:34, djsollen wrote:
> equivalent

Done.

https://codereview.chromium.org/19808007/diff/20001/src/ports/SkAtomics_andro...
File src/ports/SkAtomics_android.h (right):

https://codereview.chromium.org/19808007/diff/20001/src/ports/SkAtomics_andro...
src/ports/SkAtomics_android.h:15: #include <utils/Atomic.h>
On 2013/07/25 18:17:21, djsollen wrote:
> we should update this to
> 
> #include <cutils/atomic.h>

Done.

https://codereview.chromium.org/19808007/diff/20001/src/ports/SkMutex_none.h
File src/ports/SkMutex_none.h (right):

https://codereview.chromium.org/19808007/diff/20001/src/ports/SkMutex_none.h#...
src/ports/SkMutex_none.h:25: #define SK_DECLARE_STATIC_MUTEX(name) static
SkBaseMutex name = { }
On 2013/07/25 16:00:34, djsollen wrote:
> do you need the '{ }' here?

Hmmmm... I suppose this was just here to 0 initialize the struct (and to
parallel the construction in the _posix version). However, seeing as there is no
state in this struct, it doesn't seem like it should make a difference. On the
other hand, having the braces here also helps to prevent mis-use, it pretty much
forces the user to put a ';' after it.

[bungeman-skia, 2013-10-09 20:13:19]

Updated to have the setting in SkPostConfig/SkUserConfig.

[djsollen, 2013-10-11 15:20:18]

https://codereview.chromium.org/19808007/diff/69001/gyp/core.gyp
File gyp/core.gyp (right):

https://codereview.chromium.org/19808007/diff/69001/gyp/core.gyp#newcode100
gyp/core.gyp:100: '../src/ports',
I'm not a fan of making clients include paths have references to our src
directory.  However, I understand not wanting to add headers to our include
directory that we don't want people referencing directly.  It just doesn't seem
like the solution should be making them include our src/ports directory.  Is
there a strong argument against make the atomic and mutex headers part of our
include dir?

[bungeman-skia, 2013-12-18 15:27:52]

Committed patchset #8 manually as r12738.

[bungeman-skia, 2013-12-18 15:29:28]

https://codereview.chromium.org/19808007/diff/69001/gyp/core.gyp
File gyp/core.gyp (right):

https://codereview.chromium.org/19808007/diff/69001/gyp/core.gyp#newcode100
gyp/core.gyp:100: '../src/ports',
On 2013/10/11 15:20:18, djsollen wrote:
> I'm not a fan of making clients include paths have references to our src
> directory.  However, I understand not wanting to add headers to our include
> directory that we don't want people referencing directly.  It just doesn't
seem
> like the solution should be making them include our src/ports directory.  Is
> there a strong argument against make the atomic and mutex headers part of our
> include dir?

Done.