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

Issue 305593002: Use SkAtomics_sync on Android (Closed)

Created:
6 years, 7 months ago by mtklein_C
Modified:
6 years, 7 months ago
CC:
skia-review_googlegroups.com
Base URL:
https://skia.googlesource.com/skia.git@master
Visibility:
Public.

Description

Use SkAtomics_sync on Android Every doc I've found about using Android's atomics says, "stop". "* A handful of basic atomic operations. The appropriate pthread * functions should be used instead of these whenever possible." "... we recommend stopping from using these functions entirely. Very fortunately, GCC provides handy intrinsics functions that work with very reasonable performance and always provide a full barrier." As far as I can tell, there's no code generation change here: both the __sync atomics and the android_ atomics use full memory barriers. (And now with this all unified, it'll be easier to get the real wins by switching everything to __atomic atomics, which are like __sync atomics but allow control over memory barriers.) BUG=skia: Committed: http://code.google.com/p/skia/source/detail?r=14896

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -59 lines) Patch
M gyp/common_conditions.gypi View 1 chunk +1 line, -1 line 0 comments Download
M gyp/core.gyp View 1 chunk +0 lines, -6 lines 0 comments Download
M gyp/ports.gyp View 1 chunk +0 lines, -1 line 0 comments Download
M include/core/SkPostConfig.h View 1 chunk +0 lines, -2 lines 0 comments Download
D src/ports/SkAtomics_android.h View 1 chunk +0 lines, -49 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
mtklein
6 years, 7 months ago (2014-05-27 15:29:46 UTC) #1
djsollen
lgtm
6 years, 7 months ago (2014-05-27 15:32:54 UTC) #2
mtklein
The CQ bit was checked by mtklein@google.com
6 years, 7 months ago (2014-05-27 15:34:01 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/305593002/1
6 years, 7 months ago (2014-05-27 15:34:08 UTC) #4
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 7 months ago (2014-05-27 15:34:13 UTC) #5
commit-bot: I haz the power
Presubmit check for 305593002-1 failed and returned exit status 1. Running presubmit commit checks ...
6 years, 7 months ago (2014-05-27 15:34:13 UTC) #6
mtklein
+reed for include, still no API changes.
6 years, 7 months ago (2014-05-27 15:37:55 UTC) #7
mtklein
actually +reed for include. No API changes.
6 years, 7 months ago (2014-05-27 15:38:27 UTC) #8
reed1
lgtm -- +1 for deleting lines
6 years, 7 months ago (2014-05-27 15:38:51 UTC) #9
mtklein
The CQ bit was checked by mtklein@google.com
6 years, 7 months ago (2014-05-27 15:50:25 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/305593002/1
6 years, 7 months ago (2014-05-27 15:50:39 UTC) #11
commit-bot: I haz the power
6 years, 7 months ago (2014-05-27 15:55:38 UTC) #12
Message was sent while issue was closed.
Change committed as 14896

Powered by Google App Engine
This is Rietveld 408576698