diff --git a/src/vault/gear/scheduler-commutator.hpp b/src/vault/gear/scheduler-commutator.hpp index 4d144944a..a882fc12d 100644 --- a/src/vault/gear/scheduler-commutator.hpp +++ b/src/vault/gear/scheduler-commutator.hpp @@ -93,10 +93,13 @@ namespace gear { using std::memory_order::memory_order_relaxed; using std::memory_order::memory_order_acquire; using std::memory_order::memory_order_release; + using std::chrono_literals::operator ""us; + using std::chrono::microseconds; namespace { // Configuration / Scheduling limit Offset FUTURE_PLANNING_LIMIT{FSecs{20}}; ///< limit timespan of deadline into the future (~360 MiB max) + microseconds GROOMING_WAIT_CYCLE{70us}; ///< wait-sleep in case a thread must forcibly acquire the Grooming-Token /** convenient short-notation, also used by SchedulerService */ auto inline thisThread() { return std::this_thread::get_id(); } @@ -165,6 +168,12 @@ namespace gear { } + class ScopedGroomingGuard; + /** a scope guard to force acquisition of the GroomingToken */ + ScopedGroomingGuard requireGroomingTokenHere(); + + + void sanityCheck (ActivationEvent const& event, Time now) { @@ -279,5 +288,59 @@ namespace gear { }; + + + class SchedulerCommutator::ScopedGroomingGuard + : util::MoveOnly + { + SchedulerCommutator& commutator_; + bool handledActively_; + + bool + ensureHoldsToken() + { + if (commutator_.holdsGroomingToken(thisThread())) + return false; + while (not commutator_.acquireGoomingToken()) + std::this_thread::sleep_for (GROOMING_WAIT_CYCLE); + return true; + } + + public: + /** @warning can block indefinitely if someone hogs the token */ + ScopedGroomingGuard (SchedulerCommutator& layer2) + : commutator_(layer2) + , handledActively_{ensureHoldsToken()} + { } + + ~ScopedGroomingGuard() + { + if (handledActively_) + commutator_.dropGroomingToken(); + } + }; + + + /** + * @warning this provides very specific functionality + * required by the »Scheduler Service« to handle both + * _external_ and _internal_ calls properly. + * - whenever a thread already holds the GroomingToken, + * no further action is performed (so the cost of this + * feature is one additional atomic read on the token) + * - however, a thread coming _from the outside_ and not + * belonging to the Scheduler ecosystem is typically not + * aware of the GroomingToken altogether. The token is + * acquired, possibly incurring a *blocking wait*, and + * it is dropped transparently when leaving the scope. + */ + inline SchedulerCommutator::ScopedGroomingGuard + SchedulerCommutator::requireGroomingTokenHere() + { + return ScopedGroomingGuard(*this); + } + + + }} // namespace vault::gear #endif /*SRC_VAULT_GEAR_SCHEDULER_COMMUTATOR_H_*/ diff --git a/src/vault/gear/scheduler.hpp b/src/vault/gear/scheduler.hpp index 16696567f..3231c65ca 100644 --- a/src/vault/gear/scheduler.hpp +++ b/src/vault/gear/scheduler.hpp @@ -336,6 +336,7 @@ namespace gear { ,ManifestationID manID = ManifestationID() ,FrameRate expectedAdditionalLoad = FrameRate(25)) { + auto guard = layer2_.requireGroomingTokenHere(); // allow mutation layer1_.activate (manID); activityLang_.announceLoad (expectedAdditionalLoad); continueMetaJob (RealClock::now(), planningJob, manID); @@ -352,6 +353,7 @@ namespace gear { { bool isCompulsory = true; Time deadline = nextStart + DUTY_CYCLE_TOLERANCE; + auto guard = layer2_.requireGroomingTokenHere(); // protect allocation // place the meta-Job into the timeline... postChain ({activityLang_.buildMetaJob(planningJob, nextStart, deadline) .post() @@ -359,7 +361,6 @@ namespace gear { , deadline , manID , isCompulsory}); - ensureDroppedGroomingToken(); } @@ -574,7 +575,8 @@ namespace gear { */ inline ScheduleSpec ScheduleSpec::post() - { + { // protect allocation + auto guard = theScheduler_->layer2_.requireGroomingTokenHere(); term_ = move( theScheduler_->activityLang_ .buildCalculationJob (job_, start_,death_)); @@ -583,7 +585,6 @@ namespace gear { , death_ , manID_ , isCompulsory_}); - theScheduler_->ensureDroppedGroomingToken(); return move(*this); } @@ -732,6 +733,8 @@ namespace gear { inline void Scheduler::handleDutyCycle (Time now, bool forceContinuation) { + auto guard = layer2_.requireGroomingTokenHere(); + // consolidate queue content layer1_.feedPrioritisation(); // clean-up of outdated tasks here diff --git a/tests/vault/gear/scheduler-commutator-test.cpp b/tests/vault/gear/scheduler-commutator-test.cpp index dfa15238a..0a00b13a2 100644 --- a/tests/vault/gear/scheduler-commutator-test.cpp +++ b/tests/vault/gear/scheduler-commutator-test.cpp @@ -95,6 +95,7 @@ namespace test { { demonstrateSimpleUsage(); verify_GroomingToken(); + verify_GroomingGuard(); torture_GroomingToken(); verify_DispatchDecision(); verify_findWork(); @@ -162,6 +163,38 @@ namespace test { + /** @test verify extended logic to protect a scope + * - if the thread already holds the grooming token, nothing happens + * - otherwise, it is acquired (blocking) and dropped on exit + */ + void + verify_GroomingGuard() + { + SchedulerCommutator sched; + + // Case-1: if a thread already holds the token.... + CHECK (sched.acquireGoomingToken()); + CHECK (sched.holdsGroomingToken (thisThread())); + { + auto guard = sched.requireGroomingTokenHere(); + CHECK (sched.holdsGroomingToken (thisThread())); + }// leave scope -> nothing happens in this case + CHECK (sched.holdsGroomingToken (thisThread())); + + // Case-2: when not holding the token... + sched.dropGroomingToken(); + { + // acquire automatically (this may block) + auto guard = sched.requireGroomingTokenHere(); + CHECK (sched.holdsGroomingToken (thisThread())); + }// leave scope -> dropped automatically + CHECK (not sched.holdsGroomingToken (thisThread())); + + ___ensureGroomingTokenReleased(sched); + } + + + /** @test ensure the GroomingToken mechanism indeed creates mutual * exclusion to protected against concurrent corruption. * @remark uses lib::test::threadBenchmark() to put the test-subject diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index b9d206f23..3d32455d6 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -104754,8 +104754,8 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + @@ -105214,10 +105214,128 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- - - - + + + + + + + + +

+ dieses Prinzip könnte man aufweichen — ich halte das aber für eine schlechte Idee, denn der Scheduler ist auch so schon schwer genug zu verstehen, auch die Activity-Language beginnt schon zu »bröseln« ... gefährliche Tendenz. Meine Einschätzung ist, wir planen ehr so 10 Jobs pro Metajob-Lauf, d.h. das wären dann Größenordnung 500-1000µs Zeit, in der jedwede andere Management-Tätigkeit geblockt ist (renderjobs laufen trotzdem weiter). Sofern diese Annahme nicht zutrifft, möchte ich dafür konkrete Beweise sehen +

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

+ commit 5c6354882de1be63724221022b374bd559a499b0 +

+

+ Author: Ichthyostega <prg@ichthyostega.de> +

+

+ Date:   Wed Nov 8 20:58:32 2023 +0100 +

+

+ +

+

+     Scheduler: solve problem with transport from entrance-queue +

+

+     +

+

+     The test case "scheduleRenderJob()" -- while deliberately operated +

+

+     quite artificially with a disabled WorkForce (so the test can check +

+

+     the contents in the queue and then progress manually -- led to discovery +

+

+     of an open gap in the logic: in the (rare) case that a new task is +

+

+     added ''from the outside'' without acquiring the Grooming-Token, then +

+

+     the new task could sit in the entrace queue, in worst case for 50ms, +

+

+     until the next Scheduler-»Tick« routinely sweeps this queue. Under +

+

+     normal conditions however, each dispatch of another activity will +

+

+     also sweep the entrance queue, yet if there happens to be no other +

+

+     task right now, a new task could be stuck. +

+

+     +

+

+     Thinking through this problem also helped to amend some aspects +

+

+     of Grooming-Token handling and clarified the role of the API-functions. +

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

+ aber sie dann auch zusätzlich in das post()  eingesetzt +

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

+ ...dann erlangt er es, und behält es +

+ + + + + + + @@ -105238,6 +105356,148 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
 + + + + + + + + + + + + + + + + + + + + + + + + + + +

+ und: die Allokation findet hier statt +

+ + + + + + + + +

+     term_ = move( +

+

+       theScheduler_->activityLang_ +

+

+           .buildCalculationJob (job_, start_,death_)); +

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

+ ...eben wenn jemand von außen kommt... das ist i.d.R. nur beim seedCalcStream() der Fall +

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

+ zum einen ist das nicht genau das feste Raster, und außerdem braucht der OS-Scheduler sowiso immer etwas länger +

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

+ sage bewußt »dokumentieren«, denn diese Logik hier ist derart einfach zu implementieren, daß eigentlich kein Test notwendig wäre +

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

+ das ist nämlich unverhältnismäßig schwer im Test zu verifizieren, und steht in keinem Verhältnis zur Aufgabe +

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