-
Notifications
You must be signed in to change notification settings - Fork 198
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
Disable copy/move ctors and operator= from free_list classes #862
Disable copy/move ctors and operator= from free_list classes #862
Conversation
I've noticed some impact on the MULTI_STREAM_ALLOCATIONS_BENCH. Sometimes there appears to be some oversynchronization effect on performance. Here I ran it twice in a row, and the first run got the perf I expected, but the 4-stream version ran slower the second time:
|
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.
lgtm 👍
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.
Excellent find! I verified the C++ and Java unit tests pass with PTDS enabled after this change.
@gpucibot merge |
Fixes #861.
An implicit copy of
free_list
was being used instead of a reference, which led to duplicate allocations. This never manifested until after #851 because previously the locally modified copy of a free list was being merged into an MR-owned free list. When we removed one of the places where we merged free lists, this copy resulted in the changes to free lists being lost. This only manifested in PTDS usage, but likely would also manifest in use cases with multiple non-default streams.