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

Why use IPromise and not ValueTask for Write/Close/etc ? #48

Closed
maksimkim opened this issue May 22, 2021 · 4 comments
Closed

Why use IPromise and not ValueTask for Write/Close/etc ? #48

maksimkim opened this issue May 22, 2021 · 4 comments

Comments

@maksimkim
Copy link
Contributor

I wonder what was the reason to change channel api write methods from Task to IPromise?
If it's to have less allocations ValueTask seems better alternative given it's part of CLR today and can simplify usage with await. And it even allow to avoid allocations in most cases: https://tooslowexception.com/implementing-custom-ivaluetasksource-async-without-allocations/
I actually opened corresponding PR in original repo: Azure/DotNetty#375
Are there any benefits of IPromise?

@cuteant
Copy link
Owner

cuteant commented May 22, 2021

I like this PR: Azure/DotNetty#375,I didn't notice that before.:sweat_smile:

There is no performance benefit associated with using IPromise......

Netty's thread pool determines that IPromise cannot be completely removed, especially Codes.Http2 is a bit complicated for the use of IPromise.

To be honest, I like Orleans scheduler for single-threaded processing, it's friendly to developers

PS. My English is not good.:sleeping:

@cuteant
Copy link
Owner

cuteant commented May 22, 2021

Codes.Http2 is developed based on your http2 branch,Thanks!:raised_hands:

@maksimkim
Copy link
Contributor Author

maksimkim commented May 22, 2021

@cuteant will you consider accepting PR to SpanNetty to replace IPromise with ValueTask if I create one?

By the way, DotNetty also provides single threaded execution semantic with System.Threading.Task through custom TaskScheduler.

@cuteant
Copy link
Owner

cuteant commented May 22, 2021

@maksimkim ,:clap::clap::clap:All right!

@cuteant cuteant closed this as completed May 24, 2021
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

No branches or pull requests

2 participants