Skip to content

Commit e8e0d10

Browse files
authored
CP.1: Simplify example, show good example, expand on rationale (#1615)
"Make your code thread-safe" usually means "don't use global state." Advice to replace global state with `thread_local` state is usually misguided. https://quuxplusone.github.io/blog/2018/11/14/fiber-local-storage/
1 parent 19f2672 commit e8e0d10

File tree

2 files changed

+38
-14
lines changed

2 files changed

+38
-14
lines changed

CppCoreGuidelines.md

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13948,29 +13948,52 @@ However, over time, code fragments can turn up in unexpected places.
1394813948

1394913949
double cached_computation(double x)
1395013950
{
13951-
// bad: these two statics cause data races in multi-threaded usage
13951+
// bad: these statics cause data races in multi-threaded usage
1395213952
static double cached_x = 0.0;
1395313953
static double cached_result = COMPUTATION_OF_ZERO;
13954-
double result;
1395513954

13956-
if (cached_x == x)
13957-
return cached_result;
13958-
result = computation(x);
13959-
cached_x = x;
13960-
cached_result = result;
13961-
return result;
13955+
if (cached_x != x) {
13956+
cached_x = x;
13957+
cached_result = computation(x);
13958+
}
13959+
return cached_result;
1396213960
}
1396313961

1396413962
Although `cached_computation` works perfectly in a single-threaded environment, in a multi-threaded environment the two `static` variables result in data races and thus undefined behavior.
1396513963

13966-
There are several ways that this example could be made safe for a multi-threaded environment:
13964+
##### Example, good
13965+
13966+
struct ComputationCache {
13967+
double cached_x = 0.0;
13968+
double cached_result = COMPUTATION_OF_ZERO;
13969+
13970+
double compute(double x) {
13971+
if (cached_x != x) {
13972+
cached_x = x;
13973+
cached_result = computation(x);
13974+
}
13975+
return cached_result;
13976+
}
13977+
};
13978+
13979+
Here the cache is stored as member data of a `ComputationCache` object, rather than as shared static state.
13980+
This refactoring essentially delegates the concern upward to the caller: a single-threaded program
13981+
might still choose to have one global `ComputationCache`, while a multi-threaded program might
13982+
have one `ComputationCache` instance per thread, or one per "context" for any definition of "context."
13983+
The refactored function no longer attempts to manage the allocation of `cached_x`. In that sense,
13984+
this is an application of the Single Responsibility Principle.
13985+
13986+
In this specific example, refactoring for thread-safety also improved reusability in single-threaded
13987+
programs. It's not hard to imagine that a single-threaded program might want two `ComputationCache` instances
13988+
for use in different parts of the program, without having them overwrite each other's cached data.
13989+
13990+
There are several other ways one might add thread-safety to code written for a standard multi-threaded environment
13991+
(that is, one where the only form of concurrency is `std::thread`):
1396713992

13968-
* Delegate concurrency concerns upwards to the caller.
13969-
* Mark the `static` variables as `thread_local` (which might make caching less effective).
13970-
* Implement concurrency control, for example, protecting the two `static` variables with a `static` lock (which might reduce performance).
13971-
* Have the caller provide the memory to be used for the cache, thereby delegating both memory allocation and concurrency concerns upwards to the caller.
13993+
* Mark the state variables as `thread_local` instead of `static`.
13994+
* Implement concurrency control, for example, protecting access to the two `static` variables with a `static std::mutex`.
1397213995
* Refuse to build and/or run in a multi-threaded environment.
13973-
* Provide two implementations, one which is used in single-threaded environments and another which is used in multi-threaded environments.
13996+
* Provide two implementations: one for single-threaded environments and another for multi-threaded environments.
1397413997

1397513998
##### Exception
1397613999

scripts/hunspell/isocpp.dic

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ completers
9797
componentization
9898
composability
9999
composable
100+
ComputationCache
100101
conceptsTS
101102
cond
102103
const

0 commit comments

Comments
 (0)