-
Notifications
You must be signed in to change notification settings - Fork 4k
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
THRIFT-4327: add API to efficiently remove a single timer #1353
Conversation
@@ -69,28 +72,33 @@ class TimerManager { | |||
* | |||
* @param task The task to execute | |||
* @param timeout Time in milliseconds to delay before executing task | |||
* @return Handle of the timer, which can be used to remose the timer. |
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.
remose == remove?
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.
Done
timerManager.add(task, task->_timeout); | ||
|
||
// Remove one task and wait until the other has completed | ||
timerManager.remove(timer); |
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.
recommend calling remove a second time and validating expected behavior
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.
Done
* @throws UncancellableTaskException Specified task is already being | ||
* executed or has completed execution. | ||
*/ | ||
virtual void remove(Timer timer); |
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.
Was removal of a task insufficient in some way?
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.
The old function removes all timers executing this task, and does this inefficiently by traversing the whole list.
This new function removes a single timer in constant time [O(1)] which may be significant for large system with many timers.
This does not impact the behavior of Thrift itself, since it does not rely on timer removal.
d1fd82a
to
e37c14a
Compare
Client: C++ This closes apache#1353
Client: C++ This closes apache#1353
No description provided.