-
Notifications
You must be signed in to change notification settings - Fork 423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TOOLS/PERF: add stream tests over ucp_stream_recv_nb #2019
Conversation
Test PASSed. |
Build finished. |
Test FAILed. |
bot:mlx:retest |
Test PASSed. |
src/tools/perf/libperf.h
Outdated
@@ -79,7 +79,8 @@ enum ucx_perf_test_flags { | |||
UCX_PERF_TEST_FLAG_TAG_WILDCARD = UCS_BIT(4), /* For tag tests, use wildcard mask */ | |||
UCX_PERF_TEST_FLAG_TAG_SYNC = UCS_BIT(5), /* For tag tests, use sync send */ | |||
UCX_PERF_TEST_FLAG_VERBOSE = UCS_BIT(7), /* Print error messages */ | |||
UCX_PERF_TEST_FLAG_STREAM_RECV_DATA = UCS_BIT(8) /* For stream tests, use recv data API */ | |||
UCX_PERF_TEST_FLAG_STREAM_RECV_DATA = UCS_BIT(8), /* For stream tests, use recv data API */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
align?
src/tools/perf/ucp_tests.cc
Outdated
@@ -440,6 +460,35 @@ class ucp_perf_test_runner { | |||
return UCS_PTR_IS_PTR(data) ? UCS_OK : UCS_PTR_STATUS(data); | |||
} | |||
|
|||
ucs_status_t UCS_F_ALWAYS_INLINE | |||
recv_stream(ucp_ep_h ep, void *buf, unsigned length, ucp_datatype_t datatype, | |||
uint8_t sn) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
align
Build finished. |
Test PASSed. |
Test FAILed. |
bot:mlx:retest |
Submitted issue #2027 on failure |
Test PASSed. |
src/tools/perf/perftest.c
Outdated
@@ -372,6 +372,9 @@ static void usage(struct perftest_context *ctx, const char *program) | |||
printf(" -B Register memory with NONBLOCK flag.\n"); | |||
printf(" -C Use wildcard for tag tests.\n"); | |||
printf(" -S Use synchronous mode for tag sends.\n"); | |||
printf(" -r <mode> Stream receive mode. (recv_data)\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think recv should be the default since it symmetric for 'stream_send_nb', the 'recv_data' mode is an optimization
also can say in the help: "Receive mode for stream tests. (recv)\n"
src/tools/perf/perftest.c
Outdated
@@ -593,6 +593,17 @@ static ucs_status_t parse_test_params(ucx_perf_params_t *params, char opt, const | |||
ucs_error("Invalid option argument for -A"); | |||
return UCS_ERR_INVALID_PARAM; | |||
} | |||
case 'r': | |||
if (0 == strcmp(optarg, "recv_data")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: can do !strcmp as in the rest of the code
src/tools/perf/ucp_tests.cc
Outdated
&rlength, 0); | ||
if (ucs_unlikely(UCS_PTR_IS_ERR(rreq))) { | ||
return UCS_PTR_STATUS(rreq); | ||
} else if (UCS_PTR_STATUS(rreq) == UCS_OK) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (ucs_likely(rreq == NULL))
should be first check, also to serve as an example for users
src/tools/perf/ucp_tests.cc
Outdated
ssize_t rlength_s; | ||
|
||
do { | ||
rreq = ucp_stream_recv_nb(ep, buf, length - total, datatype, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we pass buf + total
here?
src/tools/perf/ucp_tests.cc
Outdated
size_t length; | ||
ucs_status_t status; | ||
|
||
if (ucs_likely(!UCS_PTR_IS_PTR(request))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this check needed? the function is called only if this is really a ptr. maybe should be an assertion?
+ Coding style fixes.
Build finished. |
Test PASSed. |
Test FAILed. |
#2027 |
Test PASSed. |
@yosefe good to go? |
No description provided.