diff --git a/exercises/CMakeLists.txt b/exercises/CMakeLists.txt index 36420d05..7f42f450 100644 --- a/exercises/CMakeLists.txt +++ b/exercises/CMakeLists.txt @@ -15,7 +15,6 @@ endif() # Include the exercises that (should) work on all platforms. add_subdirectory( hello ) add_subdirectory( asan ) -add_subdirectory( atomic ) add_subdirectory( basicTypes ) add_subdirectory( callgrind ) add_subdirectory( condition_variable ) diff --git a/exercises/ExerciseSchedule_AdvancedCourse.md b/exercises/ExerciseSchedule_AdvancedCourse.md index 4c3da299..72d2ed7e 100644 --- a/exercises/ExerciseSchedule_AdvancedCourse.md +++ b/exercises/ExerciseSchedule_AdvancedCourse.md @@ -44,8 +44,6 @@ Day 3 ### Race conditions (directory: [`race`](race), [cheatSheet](ExercisesCheatSheet.md#race-conditions-directory-race)) -### Atomicity (directory: [`atomic`](atomic), [cheatSheet](ExercisesCheatSheet.md#atomicity-directory-atomic)) - ### Generic programming / templates (directory: [`templates`](templates), [cheatSheet](ExercisesCheatSheet.md#generic-programming--templates-directory-templates)) As a prerequisite for variadic templates, and in case it was not covered in day 2 session diff --git a/exercises/ExercisesCheatSheet.md b/exercises/ExercisesCheatSheet.md index fa8c268e..2301c1db 100644 --- a/exercises/ExercisesCheatSheet.md +++ b/exercises/ExercisesCheatSheet.md @@ -341,13 +341,9 @@ Concurrency Exercises Typical race condition where a simple mutex and lock_guard "solves" the problem. -The second step is to look at the execution time and find out that it's not really a solution. One could then try an atomic and see the difference, although I do not introduce them in the course +The second step is to look at the execution time and find out that it's not really a solution. -### Atomicity (directory: [`atomic`](atomic)) - -Exactly the same race condition as above. Fix them using an `atomic`. - -*Optional*: Compare run times for lock and atomic solution. Those are likely not very different, as many locks are implemented using atomics. +Try then to use an `atomic` to solve the issue and compare the execution time with the lock solution. ### Condition variables (directory: [`condition_variable`](condition_variable)) diff --git a/exercises/atomic/CMakeLists.txt b/exercises/atomic/CMakeLists.txt deleted file mode 100644 index 91946368..00000000 --- a/exercises/atomic/CMakeLists.txt +++ /dev/null @@ -1,18 +0,0 @@ -# Set up the project. -cmake_minimum_required( VERSION 3.12 ) -project( atomic LANGUAGES CXX ) - -# Set up the compilation environment. -include( "${CMAKE_CURRENT_SOURCE_DIR}/../common.cmake" ) - -# Figure out how to use the platform's thread capabilities. -find_package( Threads REQUIRED ) - -# Create the user's executable. -add_executable( atomic "atomic.cpp" ) -target_link_libraries( atomic PRIVATE Threads::Threads ) - -# Create the "solution executable". -add_executable( atomic.sol EXCLUDE_FROM_ALL "solution/atomic.sol.cpp" ) -target_link_libraries( atomic.sol PRIVATE Threads::Threads ) -add_dependencies( solution atomic.sol ) diff --git a/exercises/atomic/Makefile b/exercises/atomic/Makefile deleted file mode 100644 index 7462bc2b..00000000 --- a/exercises/atomic/Makefile +++ /dev/null @@ -1,14 +0,0 @@ -PROGRAM_NAME=atomic - -all: $(PROGRAM_NAME) -solution: $(PROGRAM_NAME).sol - - -clean: - rm -f *o $(PROGRAM_NAME) *~ core $(PROGRAM_NAME).sol - -$(PROGRAM_NAME) : $(PROGRAM_NAME).cpp - ${CXX} -g -std=c++17 -O2 -pthread -Wall -Wextra -L. -o $@ $< - -$(PROGRAM_NAME).sol : solution/$(PROGRAM_NAME).sol.cpp - ${CXX} -g -std=c++17 -O2 -pthread -Wall -Wextra -L. -o $@ $< diff --git a/exercises/atomic/README.md b/exercises/atomic/README.md deleted file mode 100644 index 76b13aa2..00000000 --- a/exercises/atomic/README.md +++ /dev/null @@ -1,13 +0,0 @@ - -## Instructions - -You know this program already from "racing". -It tries to increment an integer 200 times in two threads. -Last time, we fixed the race condition using a lock, but now we'll try atomics. - -Tasks: -- Replace the counter 'a' by an atomic. - Run the program, and check for race conditions. -- Go back to 'racing', and check the execution time of the atomic vs the lock solution, - e.g. using `time ./atomic` - You might have to increase the number of tries if it completes too fast. diff --git a/exercises/atomic/atomic.cpp b/exercises/atomic/atomic.cpp deleted file mode 100644 index 713ef65f..00000000 --- a/exercises/atomic/atomic.cpp +++ /dev/null @@ -1,34 +0,0 @@ -#include -#include -#include - -int main() { - int nError = 0; - - for (int j = 0; j < 1000; j++) { - int a = 0; - - // Increment the variable a 100 times: - auto inc100 = [&a](){ - for (int i = 0; i < 100; ++i) { - a++; - } - }; - - // Run with two threads - std::thread t1(inc100); - std::thread t2(inc100); - for (auto t : {&t1,&t2}) t->join(); - - // Check - if (a != 200) { - std::cout << "Race: " << a << ' '; - nError++; - } else { - std::cout << '.'; - } - } - std::cout << '\n'; - - return nError; -} diff --git a/exercises/atomic/solution/atomic.sol.cpp b/exercises/atomic/solution/atomic.sol.cpp deleted file mode 100644 index 8fc901ef..00000000 --- a/exercises/atomic/solution/atomic.sol.cpp +++ /dev/null @@ -1,34 +0,0 @@ -#include -#include -#include - -int main() { - int nError = 0; - - for (int j = 0; j < 1000; j++) { - std::atomic a{0}; - - // Increment the variable a 100 times: - auto inc100 = [&a](){ - for (int i = 0; i < 100; ++i) { - a++; - } - }; - - // Run with two threads - std::thread t1(inc100); - std::thread t2(inc100); - for (auto t : {&t1,&t2}) t->join(); - - // Check - if (a != 200) { - std::cout << "Race: " << a << ' '; - nError++; - } else { - std::cout << '.'; - } - } - std::cout << '\n'; - - return nError; -} diff --git a/exercises/race/CMakeLists.txt b/exercises/race/CMakeLists.txt index c7415d6f..e5520481 100644 --- a/exercises/race/CMakeLists.txt +++ b/exercises/race/CMakeLists.txt @@ -13,6 +13,10 @@ add_executable( racing "racing.cpp" ) target_link_libraries( racing PRIVATE Threads::Threads ) # Create the "solution executable". -add_executable( racing.sol EXCLUDE_FROM_ALL "solution/racing.sol.cpp" ) -target_link_libraries( racing.sol PRIVATE Threads::Threads ) -add_dependencies( solution racing.sol ) +add_executable( racing.sol1 EXCLUDE_FROM_ALL "solution/racing.sol1.cpp" ) +target_link_libraries( racing.sol1 PRIVATE Threads::Threads ) +add_dependencies( solution1 racing.sol1 ) +add_executable( racing.sol2 EXCLUDE_FROM_ALL "solution/racing.sol2.cpp" ) +target_link_libraries( racing.sol2 PRIVATE Threads::Threads ) +add_dependencies( solution2 racing.sol2 ) +add_dependencies( solution solution1 solution2 ) diff --git a/exercises/race/Makefile b/exercises/race/Makefile index 4bf10084..129c7bd3 100644 --- a/exercises/race/Makefile +++ b/exercises/race/Makefile @@ -1,14 +1,17 @@ PROGRAM_NAME=racing all: $(PROGRAM_NAME) -solution: $(PROGRAM_NAME).sol +solution: $(PROGRAM_NAME).sol1 $(PROGRAM_NAME).sol2 clean: - rm -f *o $(PROGRAM_NAME) *~ core $(PROGRAM_NAME).sol + rm -f *o $(PROGRAM_NAME) *~ core $(PROGRAM_NAME).sol? $(PROGRAM_NAME) : $(PROGRAM_NAME).cpp ${CXX} ${CXXFLAGS} -g -std=c++17 -O2 -pthread -Wall -Wextra -L. -o $@ $< -$(PROGRAM_NAME).sol : solution/$(PROGRAM_NAME).sol.cpp +$(PROGRAM_NAME).sol1 : solution/$(PROGRAM_NAME).sol1.cpp + ${CXX} ${CXXFLAGS} -g -std=c++17 -O2 -pthread -Wall -Wextra -L. -o $@ $< + +$(PROGRAM_NAME).sol2 : solution/$(PROGRAM_NAME).sol2.cpp ${CXX} ${CXXFLAGS} -g -std=c++17 -O2 -pthread -Wall -Wextra -L. -o $@ $< diff --git a/exercises/race/README.md b/exercises/race/README.md index 95f6d2aa..3dc089a0 100644 --- a/exercises/race/README.md +++ b/exercises/race/README.md @@ -1,12 +1,16 @@ ## Instructions -* Compile and run the executable, see if it races - * If you have a bash shell, try `./run ./racing`, which keeps invoking the executable - until a race condition is detected -* (Optional) You can use `valgrind --tool=helgrind ./racing` to prove your assumption -* (Optional) If your operating system supports it, recompile with thread sanitizer. +The program `racing.cpp` is incrementing a shared integer many times, within several threads, which should lead to race conditions if no specific protection is used. The values of the global parameters `nThread`, `nInc` and `nRepeat` can be custommized for your own computer. + +Tasks +- Compile and run the executable, check it races. +- If you have a bash shell, try `./run ./racing`, which keeps invoking the executable until a race condition is detected. +- (Optional) You can use `valgrind --tool=helgrind ./racing` to prove your assumption +- (Optional) If your operating system supports it, recompile with thread sanitizer. With Makefile, use e.g. `make CXXFLAGS="-fsanitize=thread"` -* Use a mutex to fix the issue -* See the difference in execution time -* (Optional) Check again with `valgrind` or thread sanitizer if the problem is fixed +- Use a `std::mutex` to fix the issue. +- See the difference in execution time, for example with `time ./racing`. + You might have to increase `nRepeat` if it completes too fast, or lower it if it takes too long. +- (Optional) Check again with `valgrind` or thread sanitizer if the problem is fixed. +- Try to use `std::atomic` instead of the mutex, and compare the execution time. diff --git a/exercises/race/racing.cpp b/exercises/race/racing.cpp index 186131fe..5d8017dc 100644 --- a/exercises/race/racing.cpp +++ b/exercises/race/racing.cpp @@ -1,37 +1,40 @@ + #include #include #include /* - * This program tries to increment an integer 100 times from multiple threads. - * If the result comes out at 100*nThread, it stays silent, but it will print + * This program tries to increment an integer `nInc` times in `nThread` threads. + * If the result comes out at `nInc*nThread`, it stays silent, but it will print * an error if a race condition is detected. * If you don't see it racing, try ./run ./racing, which keeps invoking the * executable until a race condition is detected. */ -constexpr unsigned int nThread = 2; +constexpr std::size_t nThread = 10; +constexpr std::size_t nInc = 1000; +constexpr std::size_t nRepeat = 1000; int main() { int nError = 0; - for (int j = 0; j < 1000; j++) { + for (std::size_t j = 0; j < nRepeat; j++) { int a = 0; // Increment the variable a 100 times: - auto inc100 = [&a](){ - for (int i = 0; i < 100; ++i) { + auto increment = [&a](){ + for (std::size_t i = 0; i < nInc; ++i) { a++; } }; - // Start up all threads: + // Start up all threads std::vector threads; - for (unsigned int i = 0; i < nThread; ++i) threads.emplace_back(inc100); + for (std::size_t i = 0; i < nThread; ++i) threads.emplace_back(increment); for (auto & thread : threads) thread.join(); // Check - if (a != nThread * 100) { + if (a != nThread * nInc) { std::cerr << "Race detected! Result: " << a << '\n'; nError++; } diff --git a/exercises/race/solution/racing.sol.cpp b/exercises/race/solution/racing.sol.cpp deleted file mode 100644 index 8eb6fdf7..00000000 --- a/exercises/race/solution/racing.sol.cpp +++ /dev/null @@ -1,44 +0,0 @@ -#include -#include -#include -#include - -/* - * This program tries to increment an integer 100 times from multiple threads. - * If the result comes out at 100*nThread, it stays silent, but it will print - * an error if a race condition is detected. - * To run it in a loop, use - * ./run ./racing.sol - */ - -constexpr unsigned int nThread = 2; - -int main() { - int nError = 0; - - for (int j = 0; j < 1000; j++) { - int a = 0; - std::mutex aMutex; - - // Increment the variable a 100 times: - auto inc100 = [&a,&aMutex](){ - for (int i = 0; i < 100; ++i) { - std::scoped_lock lock{aMutex}; - a++; - } - }; - - // Start up all threads: - std::vector threads; - for (unsigned int i = 0; i < nThread; ++i) threads.emplace_back(inc100); - for (auto & thread : threads) thread.join(); - - // Check - if (a != nThread * 100) { - std::cerr << "Race detected! Result: " << a << '\n'; - nError++; - } - } - - return nError; -} diff --git a/exercises/race/solution/racing.sol1.cpp b/exercises/race/solution/racing.sol1.cpp new file mode 100644 index 00000000..1d883406 --- /dev/null +++ b/exercises/race/solution/racing.sol1.cpp @@ -0,0 +1,39 @@ + +#include +#include +#include +#include + +constexpr std::size_t nThread = 10; +constexpr std::size_t nInc = 1000; +constexpr std::size_t nRepeat = 1000; + +int main() { + int nError = 0; + + for (std::size_t j = 0; j < nRepeat; j++) { + int a = 0; + std::mutex aMutex; + + // Increment the variable a 100 times: + auto increment = [&a,&aMutex](){ + for (std::size_t i = 0; i < nInc; ++i) { + std::scoped_lock lock{aMutex}; + a++; + } + }; + + // Start up all threads: + std::vector threads; + for (std::size_t i = 0; i < nThread; ++i) threads.emplace_back(increment); + for (auto & thread : threads) thread.join(); + + // Check + if (a != nThread * nInc) { + std::cerr << "Race detected! Result: " << a << '\n'; + nError++; + } + } + + return nError; +} diff --git a/exercises/race/solution/racing.sol2.cpp b/exercises/race/solution/racing.sol2.cpp new file mode 100644 index 00000000..2e3f2a98 --- /dev/null +++ b/exercises/race/solution/racing.sol2.cpp @@ -0,0 +1,37 @@ + +#include +#include +#include +#include + +constexpr std::size_t nThread = 10; +constexpr std::size_t nInc = 1000; +constexpr std::size_t nRepeat = 1000; + +int main() { + int nError = 0; + + for (std::size_t j = 0; j < nRepeat; j++) { + std::atomic a{0}; + + // Increment the variable a 100 times: + auto increment = [&a](){ + for (std::size_t i = 0; i < nInc; ++i) { + a++; + } + }; + + // Start up all threads + std::vector threads; + for (std::size_t i = 0; i < nThread; ++i) threads.emplace_back(increment); + for (auto & thread : threads) thread.join(); + + // Check + if (a != nThread * nInc) { + std::cerr << "Race detected! Result: " << a << '\n'; + nError++; + } + } + + return nError; +}