Fix a bug that starves all children except an unblocked child in weight_fair TC #729
Conversation
In Stride Scheduler implementation, pass should advance to the "global pass", rather than go back to zero.
Google C++ guide states that struct members should be named "member", not "member_", unlike class members.
There is a bug that frequently blocked/unblocked children are (unfairly) more often scheduled than other children
Codecov Report
@@ Coverage Diff @@
## master #729 +/- ##
==========================================
+ Coverage 70.92% 70.99% +0.07%
==========================================
Files 220 220
Lines 14133 14103 -30
==========================================
- Hits 10024 10013 -11
+ Misses 4109 4090 -19
Continue to review full report at Codecov.
|
| return false; | ||
| } | ||
|
|
||
| child->parent_ = this; | ||
| WeightedFairTrafficClass::ChildData child_data{STRIDE1 / share, pass, child}; | ||
| ChildData child_data{STRIDE1 / share, {NextPass()}, child}; |
There was a problem hiding this comment.
Since this is initializing a struct with a union, for clarity would you be able to use designated initializers?
There was a problem hiding this comment.
Yeah, I wanted to do that, but g++5 refused with "non-trivial designated initializers are not supported" 😞
There was a problem hiding this comment.
Hrm... I was hoping that that wouldn't happen with unions because it'd be smart enough to know that one option among the union is set.
|
Thanks! |
In the current implementation of stride scheduling for weighted fair sharing, the "pass" value of unblocked (joined again) child TCs are initialized to 0. Then only that TC will be scheduled until its pass value has caught up with the that of other TCs. This caused scheduling bug reported in #717.
In addition, a minor bug, where frequently blocked TCs are more scheduled than they should be, has also been fixed (
client->remainin Fig 3 in the original paper)