Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(208)

Issue 371363004: Add SkRacy (Closed)

Created:
6 years, 5 months ago by mtklein_C
Modified:
6 years, 5 months ago
Reviewers:
mtklein, reed1
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/skia.git@master
Project:
skia
Visibility:
Public.

Description

Add SkRacy SkRacy<T> is a zero-overhead wrapper for a T, except it also silences race warnings when TSAN is running. Here we apply in several classes. In SkMatrix and SkPathRef, we use it to opportunistically cache some idempotent work. In SkPixelRef, we wrap the genIDs. We think the worst that can happen here is we'll increment the global next-genID a few times instead of once when we go to get another ID. BUG=skia: Committed: https://skia.googlesource.com/skia/+/d5e3e6ae1b3434ad1158f441902ff65f1eeaa3a7 CQ_EXTRA_TRYBOTS=tryserver.skia:Canary-Chrome-Ubuntu13.10-Ninja-x86_64-ToT-Trybot,Canary-Chrome-Win7-Ninja-x86-SharedLib_ToT-Trybot,Test-Ubuntu13.10-GCE-NoGPU-x86_64-Release-TSAN-Trybot Committed: https://skia.googlesource.com/skia/+/0b544ae222aab1c5d9122a8dfe2800451b31d979

Patch Set 1 #

Patch Set 2 : fix test #

Patch Set 3 : another approach #

Patch Set 4 : fix #

Patch Set 5 : in ctor #

Patch Set 6 : SkPathRef #

Patch Set 7 : tweak #

Total comments: 2

Patch Set 8 : oval and tracy #

Patch Set 9 : check unique define + volatile #

Unified diffs Side-by-side diffs Delta from patch set Stats (+66 lines, -45 lines) Patch
M gyp/common_conditions.gypi View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download
M include/core/SkDynamicAnnotations.h View 1 2 3 4 5 6 7 8 2 chunks +40 lines, -3 lines 0 comments Download
M include/core/SkMatrix.h View 1 2 3 4 5 6 7 3 chunks +3 lines, -2 lines 0 comments Download
M include/core/SkPathRef.h View 1 2 3 4 5 6 7 4 chunks +9 lines, -7 lines 0 comments Download
M include/core/SkPixelRef.h View 1 2 3 4 5 6 7 2 chunks +3 lines, -2 lines 0 comments Download
M src/core/SkPathRef.cpp View 1 2 3 4 5 3 chunks +10 lines, -12 lines 0 comments Download
M tools/tsan.supp View 1 2 3 4 5 1 chunk +0 lines, -18 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
mtklein
6 years, 5 months ago (2014-07-08 18:29:56 UTC) #1
reed1
lgtm https://codereview.chromium.org/371363004/diff/110001/include/core/SkDynamicAnnotations.h File include/core/SkDynamicAnnotations.h (right): https://codereview.chromium.org/371363004/diff/110001/include/core/SkDynamicAnnotations.h#newcode61 include/core/SkDynamicAnnotations.h:61: class SkRacy { nit: SkTRacy
6 years, 5 months ago (2014-07-08 19:08:55 UTC) #2
mtklein
https://codereview.chromium.org/371363004/diff/110001/include/core/SkDynamicAnnotations.h File include/core/SkDynamicAnnotations.h (right): https://codereview.chromium.org/371363004/diff/110001/include/core/SkDynamicAnnotations.h#newcode61 include/core/SkDynamicAnnotations.h:61: class SkRacy { On 2014/07/08 19:08:54, reed1 wrote: > ...
6 years, 5 months ago (2014-07-08 19:14:23 UTC) #3
mtklein
The CQ bit was checked by mtklein@google.com
6 years, 5 months ago (2014-07-08 19:14:39 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/371363004/130001
6 years, 5 months ago (2014-07-08 19:15:08 UTC) #5
commit-bot: I haz the power
Change committed as d5e3e6ae1b3434ad1158f441902ff65f1eeaa3a7
6 years, 5 months ago (2014-07-08 19:30:45 UTC) #6
mtklein
A revert of this CL has been created in https://codereview.chromium.org/377693005/ by mtklein@google.com. The reason for ...
6 years, 5 months ago (2014-07-08 21:05:40 UTC) #7
mtklein
The CQ bit was checked by mtklein@google.com
6 years, 5 months ago (2014-07-08 21:47:56 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/371363004/150001
6 years, 5 months ago (2014-07-08 21:48:53 UTC) #9
commit-bot: I haz the power
6 years, 5 months ago (2014-07-09 02:37:52 UTC) #10
Message was sent while issue was closed.
Change committed as 0b544ae222aab1c5d9122a8dfe2800451b31d979

Powered by Google App Engine
This is Rietveld 408576698