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

Async server #159

Merged

Conversation

Daniel-Svensson
Copy link
Member

@Daniel-Svensson Daniel-Svensson commented Jul 6, 2019

The goal is to make the Server part truly async in order to have it scale better while using fewer threads and to prepare it for working well when running on aspnetcore (in any form).

  • Make query async

  • Make Invoke async

  • Make submit async

  • Support returning ValueTask<..> (for invoke & queries)?
    Is it a good idea even (due to reflection boxing it seems like there are no gains)

  • Need to add corresponding tests for ValueTask<..> methods before officially supporting them

  • Go through tests and ensure they await the result on query/invoke etc

  • Consider adding new Exception DomainServiceValidationException which takes IEnumerable`

  • Add CancellationToken to async methods (usable by asp.net core …)

questions to look into

  • detect and prevent async query returning total item count via out parameter ? (it can be easy to get it wrong and assign value after await)
  • should validataion & authorisation be moved out from invoke/query/submit so it works more similar to aspnetcore filters (future thinking)
  • Should CancellationToken be passed by parameter to all async method should they in some/all cases use CancellationToken DomainServiceContext

Other changes

  • Query method is now generic so as to not require much reflection
  • User identity on DomainServiceContext now works across "await" points in the code
  • User identity is now taken from OperationContext (WCF) if three is no HttpContext.Current availible

Performance under load of simple DomainService ìn Benhmarks repo which has a query with Task:Delay(10 ms) and then returns a single City entity has increased significantly under load (tested on laptop with cooldown time between). Notice that it is burst performance which ismeasured and not steady state (it takes a while before the sync version has created enought threads to process the load).

sync_async_perf_1_city_5ms_delay

Left is master , right is this PR

Running on a workstation with low turbo boost and 10ms delay for each request gives the following numbers (but examples run at 0-2% CPU and is limited by the load testing applications):

getcities_delay_1_asyncleft_sync_right
Left is async (PR), left is master
Notice how max latency is much better for the async code


validationResults = (validationErrors == null) ? null : validationErrors.ToList();
// TODO: Remove blocking wait
Copy link
Member Author

Choose a reason for hiding this comment

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

add separate issue for this

/// <returns>The query results. May be null if there are no query results.</returns>
public virtual IEnumerable Query(QueryDescription queryDescription, out IEnumerable<ValidationResult> validationErrors, out int totalCount)
public async virtual ValueTask<ServiceQueryResult> QueryAsync<T>(QueryDescription queryDescription, CancellationToken cancellationToken)
Copy link
Member Author

@Daniel-Svensson Daniel-Svensson Aug 24, 2019

Choose a reason for hiding this comment

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

See if it makes sense to make ServiceQueryResult generic. (Can be made later)

@Daniel-Svensson Daniel-Svensson merged commit d7aa586 into OpenRIAServices:master Aug 30, 2019
@Daniel-Svensson Daniel-Svensson deleted the feature/async_server branch August 30, 2019 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant