-
Notifications
You must be signed in to change notification settings - Fork 44
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
Flush when too many handles are locked #405
Conversation
c19aa0e
to
665443b
Compare
} | ||
|
||
impl MemoryLock { | ||
/// Create a new memory lock with the given flushing threshold. | ||
pub fn new(flush_threshold: usize) -> Self { |
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 rename the function to with_threshold
?
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 don't think so, there is no other way to create it, you need a threshold to create the memory lock.
pub fn should_flush(&self) -> bool { | ||
// For now we only consider the number of handles locked, but we may consider the amount in | ||
// bytes at some point. | ||
self.locked.len() >= self.flush_threshold | ||
} |
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.
Regarding its usage, I suggest renaming to has_reach_threshold
. At least, when use in WgpuStream
, we don't flush the memory lock. The current name suggests that after this function returns true
, we should flush the memory lock.
It helps when running wgpu tests or with autotune when tensors are allocated, but the kernel is never executed.