-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
[Core] Speed up single_client_tasks_sync #41743
Conversation
Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Microbenchmark: https://buildkite.com/ray-project/release/builds/3607#018c4af2-38cb-468f-9403-f6bc9cccd1db single_client_tasks_sync = [1028.731851730263, 6.294947883118997] |
) | ||
except AttributeError: | ||
# Lazy initialization. | ||
resource_name_to_accelerator_manager = { |
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.
oh this gets cache across invocation? i thought function doesn't have attribute cached like this.
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.
Yes, this basically implements function static variable in C++.
--------- Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Currently IntelGPUAcceleratorManager.get_current_node_num_accelerators() will be called everytime we try to get accelerator manager for GPU resource. This is expensive and makes single_client_tasks_sync slower. This PR changes to only call it once. Related issue number #41695 --------- Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
Why are these changes needed?
Currently
IntelGPUAcceleratorManager.get_current_node_num_accelerators()
will be called everytime we try to get accelerator manager for GPU resource. This is expensive and makessingle_client_tasks_sync
slower. This PR changes to only call it once.Related issue number
#41695
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/
under thecorresponding
.rst
file.