UCP/PROTO: Consider RNDV_PERF_DIFF #10401
Conversation
|
Test failures should be fixed with #10397. |
| @@ -717,6 +717,45 @@ UCS_TEST_F(test_proto_perf, intersect_first) | |||
| expect_empty_range(5000, SIZE_MAX); | |||
| } | |||
|
|
|||
| UCS_TEST_F(test_proto_perf, apply_zero_bias) { | |||
There was a problem hiding this comment.
Just wondering if it makes sense to add a mock test with RNDV_PERF_DIFF and make an assertion on selected protocols
There was a problem hiding this comment.
Do you mean remove this test, and add test case to test_ucp_proto_mock_rcx?
There was a problem hiding this comment.
I don't insist on removing this test
But adding new test to test_ucp_proto_mock_rcx looks like superior alternative, because we test the entire chain
| @@ -403,6 +403,7 @@ static void ucp_proto_rndv_ctrl_variant_probe( | |||
| cfg_thresh = remote_proto->cfg_thresh; | |||
| } | |||
|
|
|||
| ucp_proto_perf_apply_bias(perf, params->perf_bias); | |||
There was a problem hiding this comment.
It seems applying this bias leads to slightly different performance numbers and therefore breaks the mock test:
log1, log2, log3
Expected protocol:
{8247, 377094, "multi-frag copy-in"},
Actual protocol:
8247-363935 multi-frag copy-in
Could it be due to adding this bias? If so, we should update here the numbers in mock test as well
There was a problem hiding this comment.
Yes, it is because of adding the bias. UCX selects rendezvous earlier with this changes.
In fact none of the test failure can be fixed by that PR. They are triggered by changed perf calculation logic I guess |
…der-rndv-perf-diff
rakhmets
force-pushed
the
topic/proto-consider-rndv-perf-diff
branch
from
aaa08bd to
db2470a
Compare
What?
Applies
UCX_RNDV_PERF_DIFFsetting effect for protov2.Cherry-picked from #10292.
Why?
This control effect was removed during introducing perf-factors logic. This PR brings it back.
That how it looks like with that patch:
