From cbaa17f4a597477f9c880f45963a86fcf0aaf60b Mon Sep 17 00:00:00 2001 From: David Chamont Date: Sun, 29 Sep 2024 22:11:10 +0200 Subject: [PATCH] Integrate atomic exercise as a final task in race exercise. --- exercises/CMakeLists.txt | 1 - exercises/atomic/CMakeLists.txt | 18 ---------- exercises/atomic/Makefile | 14 -------- exercises/atomic/README.md | 13 ------- exercises/atomic/atomic.cpp | 34 ------------------ exercises/atomic/solution/atomic.sol.cpp | 34 ------------------ exercises/race/CMakeLists.txt | 10 ++++-- exercises/race/Makefile | 9 +++-- exercises/race/README.md | 20 ++++++----- exercises/race/racing.cpp | 21 ++++++----- exercises/race/solution/racing.sol.cpp | 44 ------------------------ exercises/race/solution/racing.sol1.cpp | 39 +++++++++++++++++++++ exercises/race/solution/racing.sol2.cpp | 37 ++++++++++++++++++++ 13 files changed, 113 insertions(+), 181 deletions(-) delete mode 100644 exercises/atomic/CMakeLists.txt delete mode 100644 exercises/atomic/Makefile delete mode 100644 exercises/atomic/README.md delete mode 100644 exercises/atomic/atomic.cpp delete mode 100644 exercises/atomic/solution/atomic.sol.cpp delete mode 100644 exercises/race/solution/racing.sol.cpp create mode 100644 exercises/race/solution/racing.sol1.cpp create mode 100644 exercises/race/solution/racing.sol2.cpp 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/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; +}