Extract the attribution duty cycle into DutyCycle - #136
Merged
Merged
Conversation
The idle, warm-up and burst schedule that drove per-mod attribution now lives in a class of its own, so a second feature can run on the same schedule instead of keeping a copy that would drift. DutyCycle owns the clamp, the interval, the warm-up and the burst length, and says what each tick is for: idle, start, warm-up, sample or last sample. TickAttribution folds a tree on the sample steps and keeps the rest of what it had, with the same constructor, the same Apply and the same properties, so AttributionMetrics and the commands are untouched. Behaviour is unchanged. The schedule tests moved to DutyCycleTests, where they run against the schedule alone, and the attribution tests that check what attribution does with each step stay where they were. The two mutation patterns that pointed at the moved code now point at DutyCycle.cs.
The schedule is the part every burst of every measurement will now run on, so each way it can drift gets a mutation in tools/mutation-check.sh: the idle branch, counting seconds rather than ticks, the interval boundary, the burst length boundary, the end of a burst, what Apply drops, and the three clamps. Attribution's own use of the steps gets two more, and a third for a reload that has to take the half-folded burst with it. Four tests back them. A reload mid-interval restarts the interval from the reload, which nothing checked before: the mutant that kept the idle time already counted survived the old suite. The interval is counted in seconds, so the class itself fails when it counts ticks. A reload that leaves the cycle on drops the burst in progress the way a switch-off does. And attribution drops what a cut-short burst had folded, which used to be covered only because the burst end and the reload shared one Restart.
The start of a burst reset the idle time, the sample count and the warm-up flag that Restart had already reset when the previous burst ended or the cycle was applied, so neither copy could be pinned by a mutation. Restart now owns them, with a mutation each. The cap and floor tests compared against the constants themselves, so a moved cap passed; they now assert the 300 ticks and 1 second the README documents. The burst end's ClearBurst gets the mutation Apply's already had, and the duty cycle mutations sit in one block.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Per-mod attribution runs on a duty cycle: by default it waits 10 seconds, then profiles a burst of 10 ticks after discarding one warm-up tick. That cycle lived inside
TickAttribution. The Stratum behavior timings (#6) need the same schedule, so it moves into aDutyCycleclass both features can use. Two copies of a state machine would drift apart. This is a pure refactoring: attribution's behaviour, config, commands, logs and served metrics do not change.What moved
DutyCycleowns the schedule and nothing else: the clamps, the idle, warm-up and burst counters, and the enabled, interval and burst-length state.Apply(enabled, burstTicks, intervalSeconds)reconfigures it and drops a burst in progress.OnTick(elapsedSeconds)says what the current tick is:Idle,Start,WarmUp,SampleorLastSample.TickAttributionkeeps the folding. It folds the tree onSampleandLastSample, publishes onLastSample, and itsProfilingis the cycle'sInBurst.AttributionMetrics,PulseCommands, the commands and the logs are untouched.StartandWarmUphave no caller beyond attribution's step check today. They map onto what the Stratum feature will do: request its recording lease onStart, take the first snapshot onWarmUp, and take the second snapshot and release the lease onLastSample.How it was checked
TickAttributionand the new pair on 20,000 random sequences of 400 operations, about 8,000,000 comparisons. Every burst and every observable property matched after every operation.AttributionMetricsswitches attribution off and forces the profiler off on any exception, so nothing reads that state.Restartalready does. Neither copy could be pinned by a mutation, soRestartnow owns them, with one mutation each.Tests
TickAttributiontests, 4 moved toDutyCycleTests(15 tests) with their assertions, and 21 remain, one of them new. No test was deleted.tools/mutation-check.shkills 135 of 135 mutations, with none inert:DutyCycle.cs;Restartresets, the documented burst cap, and the clear at the end of a published burst.