Skip to content
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

Adds IEventExecutor.Schedule, proper cancellation of scheduled tasks #80

Merged
merged 1 commit into from
Apr 6, 2016

Conversation

nayato
Copy link
Member

@nayato nayato commented Apr 4, 2016

Motivation:
Allow for less GC when scheduling if Task-based API is not required. Allow for proper cleanup when canceling scheduled task so that associated resources can be cleaned up early.

Modifications:

  • Moved out scheduled task implementations from AbstractScheduledEventExecutor
  • Introduced IScheduledTask and aligned IScheduledRunnable
  • Implemented removal of scheduled task upon its cancellation

Result:
Scheduled tasks that normally never fire (e.g. timeout handling) do not cause resources to be held up unnecessarily in memory for longer period of time if not necessary.

{
if (this.cancellationToken.IsCancellationRequested)
{
this.Promise.TrySetCanceled();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return from here

@nayato nayato force-pushed the cancel-schedule branch 2 times, most recently from 085f4b9 to 6fb4ba4 Compare April 5, 2016 22:35

protected abstract void Execute();

bool TrySetUncancelable()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please re-use this method above if possible

@mtuchkov
Copy link
Contributor

mtuchkov commented Apr 6, 2016

Looks good

Motivation:
Allow for less GC when scheduling if Task-based API is not required. Allow for proper cleanup when canceling scheduled task so that associated resources can be cleaned up early.

Modifications:
- Moved out scheduled task implementations from AbstractScheduledEventExecutor
- Introduced IScheduledTask and aligned IScheduledRunnable
- Implemented removal of scheduled task upon its cancellation
- Extra minor fixes
 - IByteBuffer.ToString(Encoding) honoring ArrayOffset
 - InternalLoggerFactory creating default logging factory lazily

Result:
Scheduled tasks that normally never fire (e.g. timeout handling) do not cause resources to be held up unnecessarily in memory for longer period of time if not necessary.
@nayato nayato merged commit 1986553 into Azure:dev Apr 6, 2016
@nayato nayato deleted the cancel-schedule branch April 10, 2016 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants