Commit Graph

10 Commits

Author SHA1 Message Date
Lee Salzman
ee1c73fc1b tweak SkOnce.h header to work around issues with std::atomic::compare_exchange_strong
BUG=skia:

Change-Id: Id29c47cb4bec6a6d70d6514d18fa4ab9bd31b1e2
Reviewed-on: https://skia-review.googlesource.com/6200
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Mike Klein <mtklein@chromium.org>
2016-12-16 22:21:27 +00:00
mtklein
b37c68ad42 Simplify implementation of SkOnce to not need so many comments.
I think this version reads more clearly, and the key invariants are
expressed in code rather than comments:
   - race losers always go through an acquire
   - we never exit the function unless fState is Done

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1951013004

Review-Url: https://codereview.chromium.org/1951013004
2016-05-04 13:57:30 -07:00
mtklein
e86e51fe7e Add reminders that these classes have constexpr default constructors.
We can do this now that we can write "constexpr".

GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1936653002

Review-Url: https://codereview.chromium.org/1936653002
2016-04-29 13:58:18 -07:00
mtklein
650f9e9a26 SkOnce: 2 bytes -> 1 byte
This uses the same logic we worked out for SkOncePtr to reduce
the memory footprint of SkOnce from a done byte and lock byte
to a single 3-state byte:

  - NotStarted: no thread has tried to run fn() yet
  - Active:     a thread is running fn()
  - Done:       fn() is complete

Threads which see Done return immediately.
Threads which see NotStarted try to move to Active, run fn(), then move to Done.
Threads which see Active spin until the active thread moves to Done.

This additionally fixes a too-weak memory order bug in SkOncePtr,
and adds a big note to explain.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1904483003

Committed: https://skia.googlesource.com/skia/+/df02d338be8e3c1c50b48a3a9faa582703a39c07

Review URL: https://codereview.chromium.org/1904483003
2016-04-20 13:49:15 -07:00
mtklein
9134686fd9 Revert of SkOnce: 2 bytes -> 1 byte (patchset #4 id:60001 of https://codereview.chromium.org/1904483003/ )
Reason for revert:
bust the roll

Original issue's description:
> SkOnce: 2 bytes -> 1 byte
>
> This uses the same logic we worked out for SkOncePtr to reduce
> the memory footprint of SkOnce from a done byte and lock byte
> to a single 3-state byte:
>
>   - NotStarted: no thread has tried to run fn() yet
>   - Active:     a thread is running fn()
>   - Done:       fn() is complete
>
> Threads which see Done return immediately.
> Threads which see NotStarted try to move to Active, run fn(), then move to Done.
> Threads which see Active spin until the active thread moves to Done.
>
> This additionally fixes a too-weak memory order bug in SkOncePtr,
> and adds a big note to explain.
>
> BUG=skia:
> GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1904483003
>
> Committed: https://skia.googlesource.com/skia/+/df02d338be8e3c1c50b48a3a9faa582703a39c07

TBR=herb@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review URL: https://codereview.chromium.org/1898413004
2016-04-20 13:02:09 -07:00
mtklein
df02d338be SkOnce: 2 bytes -> 1 byte
This uses the same logic we worked out for SkOncePtr to reduce
the memory footprint of SkOnce from a done byte and lock byte
to a single 3-state byte:

  - NotStarted: no thread has tried to run fn() yet
  - Active:     a thread is running fn()
  - Done:       fn() is complete

Threads which see Done return immediately.
Threads which see NotStarted try to move to Active, run fn(), then move to Done.
Threads which see Active spin until the active thread moves to Done.

This additionally fixes a too-weak memory order bug in SkOncePtr,
and adds a big note to explain.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1904483003

Review URL: https://codereview.chromium.org/1904483003
2016-04-20 10:54:54 -07:00
mtklein
d9dd428211 Modernize and trim down SkOnce.
The API and implementation are very much simplified.
You may not want to bother reading the diff.

As is our trend, SkOnce now uses <atomic> directly.

Member initialization means we don't need SK_DECLARE_STATIC_ONCE.
SkSpinlock already works this same way.

All uses of the old API taking an external bool* and Lock* were pessimal,
so I have not carried this sort of API forward.  It's simpler, faster,
and more space-efficient to always use this single SkOnce class interface.

SkOnce weighs 2 bytes: a done bool and an SkSpinlock, also a bool internally.
This API refactoring opens up the opportunity to fuse those into a single
three-state byte if we'd like.

No public API changes.
TBR=reed@google.com

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1894893002

Review URL: https://codereview.chromium.org/1894893002
2016-04-18 08:09:11 -07:00
bungeman
2c4bd0798e Convert SkRefCnt to std::atomic.
This enables removing the more complicated atomic shims from SkAtomics.h.

TBR=reed
This doesn't actually change any API.

CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-TSAN-Trybot,Test-Ubuntu-GCC-Golo-GPU-GT610-x86_64-Release-TSAN-Trybot

Review URL: https://codereview.chromium.org/1867863002
2016-04-08 06:58:52 -07:00
mtklein
15923c9e47 Modernize SkSpinlock.
- Use std::atomic directly.
 - No more need for SkPODSpinlock or SK_DECLARE_STATIC_SPINLOCK.

Now simple code like this works as you'd hope:
    static SkSpinlock gLock;

That is, it starts unlocked and there's no static initializer.

std::atomic_flag would make this terser and standard-guaranteed,
but ATOMIC_FLAG_INIT caused not-yet-implemented errors on MSVC 2013.
The generated code for this approach is identical.

It appears the implicit constructor is constexpr when all the member
initializers are.  I'm hoping this way of producing constexpr constructors
without typing "constexpr" gives us a way to eliminate more SkFoo / SkBaseFoo
distinctions and SK_DECLARE_STATIC_FOO.  This was certainly the easiest.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1734383002

Review URL: https://codereview.chromium.org/1734383002
2016-02-29 10:14:38 -08:00
herb
62a69c26b3 Move Mutexy things to private.
There is no API change.

TBR=reed@google.com

BUG=skia:

Review URL: https://codereview.chromium.org/1368333004
2015-09-29 11:47:45 -07:00