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

Implement manual state machine#4687

Merged
rynowak merged 1 commit intodevfrom
state-machine
Aug 1, 2016
Merged

Implement manual state machine#4687
rynowak merged 1 commit intodevfrom
state-machine

Conversation

@rynowak
Copy link
Member

@rynowak rynowak commented May 19, 2016

This change is a rewrite of the action invoker into a manual state machine. This makes techempower plaintext go from 245krps -> 265krps.

@rynowak
Copy link
Member Author

rynowak commented May 19, 2016

/cc @javiercn

Please consider this new code, looking at the previous code might be helpful to understand, but this diff won't be, they are too different.

_resourceExecutedContext = new ResourceExecutedContext(_resourceExecutingContext, _filters)
{
Canceled = true,
Result = _resourceExecutingContext.Result, // Can be null
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment incorrect. Have just asserted that Result is non-null.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@rynowak rynowak added this to the 1.0.1 milestone May 24, 2016
}

case State.ResultAsyncBegin:
{
Copy link
Member

Choose a reason for hiding this comment

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

Debug.Assert(_resultExecutingContext != null)

@javiercn
Copy link
Member

@javiercn
Copy link
Member

Overall looks good. Functionally everything looks alright compared to the old code (functionally equivalent). There are some small things we can improve (more Debug.Assert) and some comments at certain points of the state transitions to clarify what the algorithm does, as its hard to follow from the state transitions

@rynowak
Copy link
Member Author

rynowak commented Jul 19, 2016

@javiercn updated


private async Task InvokeActionFilterAsync()
private async Task InvokeActionMethodAsync()
{
Copy link
Member

Choose a reason for hiding this comment

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

Does this include the fix for public object Action(){ return Content("Hello world")!} ?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is up to date as of when I updated the PR. If this is missing then I'll address it when I rebase.

@javiercn
Copy link
Member

:shipit:

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.

4 participants