Extend nixl_test with multi-threading - #200
Merged
Merged
Conversation
Nixl_test shares the same code for both target and initiator which caused to acquire conditional checks for the process type all around the code decreasing its readability and maintainability. Split the code into two process type-specific functions. Refactor code where it makes sense to simplify: change loop that verifies memory contents and which can never terminate unless the contents match to a bounded loop with timeouts, simplify unnecessary complicated implementations of some loops, user constructors instead of manually setting structure fields where possible, extract commonly used literals into defines or static const vars. Update the code to use C++ algorithms, data structures and memory management primitives from their C counterparts. Signed-off-by: Vlad Buslov <vladbu@nvidia.com>
With refactoring from previous commit it becomes trivial to extend the test with multi-threading, so just take the parts that makes sense to execute concurrently (everything past agent creation and backend initialization) from both initiator and target functions and extract them into dedicated helpers. The only tricky part is notification management in initiator - since NIXL API doesn't provide a way to specify how many maximum total notifications the caller of getNotifs() is ready to receive so it is also impossible to guarantee that every thread will receive exactly one. Extend the per-thread code to store serdes objects instantiated from notifications in a shared vector protected with mutex and indexed by the thread id to remove the requirement for every thread to receive exactly one. Signed-off-by: Vlad Buslov <vladbu@nvidia.com>
|
👋 Hi w1ldptr! Thank you for contributing to ai-dynamo/nixl. Your PR reviewers will review your contribution then trigger the CI to test your changes. 🚀 |
Contributor
|
/ok to test fc4df55 |
tstamler
approved these changes
May 7, 2025
vvenkates27
pushed a commit
that referenced
this pull request
May 8, 2025
* Cleanup nixl_test Nixl_test shares the same code for both target and initiator which caused to acquire conditional checks for the process type all around the code decreasing its readability and maintainability. Split the code into two process type-specific functions. Refactor code where it makes sense to simplify: change loop that verifies memory contents and which can never terminate unless the contents match to a bounded loop with timeouts, simplify unnecessary complicated implementations of some loops, user constructors instead of manually setting structure fields where possible, extract commonly used literals into defines or static const vars. Update the code to use C++ algorithms, data structures and memory management primitives from their C counterparts. Signed-off-by: Vlad Buslov <vladbu@nvidia.com> * Implement multi-threaded execution in nixl_test With refactoring from previous commit it becomes trivial to extend the test with multi-threading, so just take the parts that makes sense to execute concurrently (everything past agent creation and backend initialization) from both initiator and target functions and extract them into dedicated helpers. The only tricky part is notification management in initiator - since NIXL API doesn't provide a way to specify how many maximum total notifications the caller of getNotifs() is ready to receive so it is also impossible to guarantee that every thread will receive exactly one. Extend the per-thread code to store serdes objects instantiated from notifications in a shared vector protected with mutex and indexed by the thread id to remove the requirement for every thread to receive exactly one. Signed-off-by: Vlad Buslov <vladbu@nvidia.com> --------- Signed-off-by: Vlad Buslov <vladbu@nvidia.com>
This was referenced Jul 22, 2026
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
3 participants
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.
First commit performs clean-ups and preparatory refactorings, second one implements multi-threading execution.