From 08c3e76f14ff5779e8ffff1be1e57edc3d149fb2 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Sat, 7 Oct 2023 03:25:39 +0200 Subject: [PATCH] Library: identified design challenges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a close look, the wrapper design as pursued here turns out to be prone to insidious data race problems. This was true also for the existing solution, but becomes more clear due to the precise definitions from the C++ standard. This is a confusing situation, because these races typically do not materialise in practice; due to the latency of the OS scheduler the new thread starts invoking user code at least 100µs after the Wrapper object is fully constructed (typically more like 500µs, which is a lot) The standard case (lib::Thread) in its current form is correct, but borderline to undefined behaviour, and any initialisation of members in a derived class would be off limits (the thread-wrapper should not be used as baseclass, rather as member) --- src/lib/test/microbenchmark.hpp | 10 +- .../library/thread-wrapper-lifecycle-test.cpp | 20 +- wiki/thinkPad.ichthyo.mm | 201 +++++++++++++++++- 3 files changed, 218 insertions(+), 13 deletions(-) diff --git a/src/lib/test/microbenchmark.hpp b/src/lib/test/microbenchmark.hpp index 6d16d5e9c..3b82d3b1d 100644 --- a/src/lib/test/microbenchmark.hpp +++ b/src/lib/test/microbenchmark.hpp @@ -68,7 +68,7 @@ namespace test{ namespace { constexpr size_t DEFAULT_RUNS = 10'000'000; - constexpr double SCALE = 1e6; // Results are in µ-sec + using CLOCK_SCALE = std::micro; // Results are in µ-sec } @@ -83,12 +83,12 @@ namespace test{ benchmarkTime (FUN const& invokeTestLoop, const size_t repeatCnt = DEFAULT_RUNS) { using std::chrono::system_clock; - using Dur = std::chrono::duration; + using Dur = std::chrono::duration; auto start = system_clock::now(); invokeTestLoop(); Dur duration = system_clock::now () - start; - return duration.count()/(repeatCnt) * SCALE; + return duration.count()/(repeatCnt); }; @@ -154,7 +154,7 @@ namespace test{ threadBenchmark(FUN const& subject, const size_t repeatCnt = DEFAULT_RUNS) { using std::chrono::system_clock; - using Dur = std::chrono::duration; + using Dur = std::chrono::duration; // the test subject gets the current loop-index and returns a checksum value ASSERT_VALID_SIGNATURE (decltype(subject), size_t(size_t)); @@ -194,7 +194,7 @@ namespace test{ checksum += thread.checksum; } - double micros = sumDuration.count() / (nThreads * repeatCnt) * SCALE; + double micros = sumDuration.count() / (nThreads * repeatCnt); return std::make_tuple (micros, checksum); } diff --git a/tests/library/thread-wrapper-lifecycle-test.cpp b/tests/library/thread-wrapper-lifecycle-test.cpp index 9022747f2..56230fa45 100644 --- a/tests/library/thread-wrapper-lifecycle-test.cpp +++ b/tests/library/thread-wrapper-lifecycle-test.cpp @@ -30,6 +30,7 @@ #include "lib/iter-explorer.hpp" #include "lib/scoped-collection.hpp" #include "lib/test/microbenchmark.hpp" +#include "lib/test/diagnostic-output.hpp" #include #include @@ -40,6 +41,7 @@ using std::atomic_uint; using std::this_thread::yield; using std::this_thread::sleep_for; using std::chrono::microseconds; +using std::chrono::system_clock; namespace lib { @@ -49,6 +51,8 @@ namespace test{ const uint NUM_THREADS = 200; const uint REPETITIONS = 10; + + using CLOCK_SCALE = std::micro; // Results are in µ-sec } @@ -73,11 +77,19 @@ namespace test{ void defaultWrapperLifecycle() { - atomic_uint i{0}; - Thread thread("counter", [&]{ ++i; }); // bind a λ and launch thread - while (thread) yield(); // ensure thread has finished and detached + using Dur = std::chrono::duration; + using Point = system_clock::time_point; + Point threadStart; + Point afterCtor; + Thread thread("lifecycle", [&]{ + threadStart = system_clock::now(); + }); + afterCtor = system_clock::now(); + while (thread) yield(); - CHECK (i == 1); // verify the effect has taken place + double offset = Dur{threadStart - afterCtor}.count(); +SHOW_EXPR(offset) + CHECK (offset > 0); UNIMPLEMENTED ("demonstrate state change"); } diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index f50119019..29029bfff 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -65253,7 +65253,8 @@ - + + @@ -65265,7 +65266,7 @@ - + @@ -65274,9 +65275,10 @@

+ - + @@ -65285,6 +65287,7 @@

+ @@ -65312,6 +65315,196 @@ + + + + + + + + + + +

+ ...sonst wäre das ganze Unterfangen theoretischer Natur; nur dadurch wird es interessant, Storage zu verwalten und einen Zugang freizuschalten +

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +

+ ⟹ Initialisierung von Policy-Bausteinen ist nicht möglich +

+ + + + + + + + + + +

+ (seufz) +

+

+ streng genommen könnte die Thread-Funktion bereits mit der Initialisierung des lib::Result beginnen, während ihr dann die default-Initialisierung dazwischenfunkt +

+ + + + + + + + + + + + + + + + + + + + + + +

+ da erfahrungsgemäß der OS-Scheduler immer eine gewisse Latenz hat, bis ein Thread tatsächlich ausführbar wird +

+ + + + + + + + + + + + + + + + + + +

+ und trotzdem: das ist kein Gegenbeweis +

+ + + + + + + + + + + + + +

+ man könnte das Thema entschärfen, +

+

+ indem man das Thread-Handle zunächst leer initialisiert +

+

+ und dann erst ganz oben im ctor-chain den Thread startet +

+

+ und per move-assign auf das Handle schiebt. +

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -81752,7 +81945,7 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- +