Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Fast path Consume when already complete#521

Merged
halter73 merged 1 commit into
aspnet:devfrom
benaadams:consume-faster
Feb 18, 2016
Merged

Fast path Consume when already complete#521
halter73 merged 1 commit into
aspnet:devfrom
benaadams:consume-faster

Conversation

@benaadams
Copy link
Copy Markdown
Contributor

Fast path Consume when content complete by not using async state machine (is normally complete)

From #519

@benaadams benaadams changed the title Fast path async consume when content complete Fast path Consume when complete Dec 23, 2015
@benaadams benaadams changed the title Fast path Consume when complete Fast path Consume when already complete Dec 23, 2015
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand this. Why not put his call to ReadAsyncImplementation inside of ConsumeAwaited? You already do this inside the while condition.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah is a bit weird; enough to make me look at it for a good few minutes before working out why.

Is because the non-completed one passes in the current task so is to match that. Will have a think on how it can be made better.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Swapped the parameters; and made the task have a null default. Better?

@halter73
Copy link
Copy Markdown
Member

Will have to be updated post ValueTask merge, but aside from my one comment this is good to 🚢

@benaadams
Copy link
Copy Markdown
Contributor Author

This is a a bit fiddly with the ValueTask; merge to rebase, will take a little bit of time.

@benaadams
Copy link
Copy Markdown
Contributor Author

Closing this; I think I'd need to start again; too much code has changed in the merge. Likely to make mistakes.

@benaadams
Copy link
Copy Markdown
Contributor Author

Rebased and reworked to work with the ValueTask changes

@halter73
Copy link
Copy Markdown
Member

:shipit:

@halter73 halter73 merged commit e533d5d into aspnet:dev Feb 18, 2016
@benaadams benaadams deleted the consume-faster branch May 10, 2016 11:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants