Avoid hang in OncePtr test with --threads 1

Avoid hang in OncePtr test with --threads 1

Avoid hang in OncePtr test when using "dm --threads 1".

The test will hang the threads until sk_num_cores() threads have run
the code. This requires that sk_num_cores() threads to be run in
parallel, which the global thread pool will not do if the thread count
is smaller than sk_num_cores().

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/7f97a76b805b4cc05f83a965063767f0256ea270

[Kimmo Kinnunen, 2015-10-26 10:09:08]

kkinnunen@nvidia.com changed reviewers:
+ herb@google.com, mtklein@google.com

[Kimmo Kinnunen, 2015-10-26 10:09:08]

Just a strawman, feel free to reformulate.

[mtklein, 2015-10-26 13:36:37]

https://codereview.chromium.org/1419593004/diff/1/tests/OncePtrTest.cpp
File tests/OncePtrTest.cpp (right):

https://codereview.chromium.org/1419593004/diff/1/tests/OncePtrTest.cpp#newco...
tests/OncePtrTest.cpp:26: SkAtomic<int> force_a_race(sk_num_cores());
Does this work correctly if we change the initial count from sk_num_cores() to
FLAGS_threads?

https://codereview.chromium.org/1419593004/diff/1/tests/OncePtrTest.cpp#newco...
tests/OncePtrTest.cpp:32: SkAutoTDelete<int> n(once.get(create));
Pretty sure ~SkOncePtr deletes the pointer and this is a double delete.

[herb_g, 2015-10-26 14:32:56]

https://codereview.chromium.org/1419593004/diff/1/tests/OncePtrTest.cpp
File tests/OncePtrTest.cpp (right):

https://codereview.chromium.org/1419593004/diff/1/tests/OncePtrTest.cpp#newco...
tests/OncePtrTest.cpp:26: SkAtomic<int> force_a_race(sk_num_cores());
On 2015/10/26 13:36:37, mtklein wrote:
> Does this work correctly if we change the initial count from sk_num_cores() to
> FLAGS_threads?

I agree with Mike on this. I don't think the new guard above is not needed.

[mtklein, 2015-10-26 14:50:55]

On 2015/10/26 14:32:56, herb_g wrote:
> https://codereview.chromium.org/1419593004/diff/1/tests/OncePtrTest.cpp
> File tests/OncePtrTest.cpp (right):
> 
>
https://codereview.chromium.org/1419593004/diff/1/tests/OncePtrTest.cpp#newco...
> tests/OncePtrTest.cpp:26: SkAtomic<int> force_a_race(sk_num_cores());
> On 2015/10/26 13:36:37, mtklein wrote:
> > Does this work correctly if we change the initial count from sk_num_cores()
to
> > FLAGS_threads?
> 
> I agree with Mike on this. I don't think the new guard above is not needed.

One too many negatives in there?  We agree the guard can be replaced with
sk_num_cores() -> FLAGS_threads right?

[Kimmo Kinnunen, 2015-10-27 06:44:42]

https://codereview.chromium.org/1419593004/diff/1/tests/OncePtrTest.cpp
File tests/OncePtrTest.cpp (right):

https://codereview.chromium.org/1419593004/diff/1/tests/OncePtrTest.cpp#newco...
tests/OncePtrTest.cpp:26: SkAtomic<int> force_a_race(sk_num_cores());
On 2015/10/26 13:36:37, mtklein wrote:
> Does this work correctly if we change the initial count from sk_num_cores() to
> FLAGS_threads?

Sure..

https://codereview.chromium.org/1419593004/diff/1/tests/OncePtrTest.cpp#newco...
tests/OncePtrTest.cpp:32: SkAutoTDelete<int> n(once.get(create));
On 2015/10/26 13:36:37, mtklein wrote:
> Pretty sure ~SkOncePtr deletes the pointer and this is a double delete.

I see. I didn't think this through, sorry.

[Kimmo Kinnunen, 2015-10-27 06:45:13]

Description was changed from

==========
Avoid hang in OncePtr test with --threads 1

Avoid hang in OncePtr test when using "dm --threads 1".

The test will hang the threads until sk_num_cores() threads have run
the code. This requires that sk_num_cores() threads to be run in
parallel, which the global thread pool will not do if the thread count
is smaller than sk_num_cores().

Also fixes a memory leak.

BUG=skia:
==========

to

==========
Avoid hang in OncePtr test with --threads 1

Avoid hang in OncePtr test when using "dm --threads 1".

The test will hang the threads until sk_num_cores() threads have run
the code. This requires that sk_num_cores() threads to be run in
parallel, which the global thread pool will not do if the thread count
is smaller than sk_num_cores().

BUG=skia:
==========

[Kimmo Kinnunen, 2015-10-27 14:23:12]

https://codereview.chromium.org/1419593004/diff/20001/tests/OncePtrTest.cpp
File tests/OncePtrTest.cpp (right):

https://codereview.chromium.org/1419593004/diff/20001/tests/OncePtrTest.cpp#n...
tests/OncePtrTest.cpp:9: #include "SkCommonFlags.h"
forgot to remove this..

[Kimmo Kinnunen, 2015-11-04 11:10:40]

So, ping..

[mtklein, 2015-11-04 14:18:30]

The CQ bit was checked by mtklein@google.com

[mtklein, 2015-11-04 14:18:30]

lgtm

[commit-bot: I haz the power, 2015-11-04 14:18:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1419593004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1419593004/40001

[commit-bot: I haz the power, 2015-11-04 14:30:21]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/7f97a76b805b4cc05f83a965063767f0256ea270