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 |
| @@ -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.
don't we need to apply it for RTS only? E.g. now it is applied for RTR as well making it more preferable than get_zcopy
There was a problem hiding this comment.
I thought it is done in this way according to this comment: #10292 (comment).
ucp_proto_rndv_ctrl_init_params_t::perf_bias is controlled by UCX_RNDV_PERF_DIFF in RTS protocol, and ucp_proto_rndv_ctrl_init_params_t::perf_bias is set to zero in RTR protocol.
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:
