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

Enhance AsyncLock to support passing state. #554

Merged
merged 1 commit into from
Jun 6, 2018
Merged

Enhance AsyncLock to support passing state. #554

merged 1 commit into from
Jun 6, 2018

Conversation

danielcweber
Copy link
Collaborator

No description provided.

@danielcweber
Copy link
Collaborator Author

Up to discussion: Should the public API be enhanced and the new internal method be made public?

@akarnokd
Copy link
Collaborator

I'm not convinced this is much of a win. the object state requires structs to be boxed which is equivalent with having a capture of a struct inside a freshly allocated delegate.

@danielcweber
Copy link
Collaborator Author

danielcweber commented May 30, 2018

Right, structs would need to be boxed, but I found most of the time, if something would need to be passed to AsyncLock, it's a class. Besides, if creation of a closure and a delegate could be avoided, it's still a saving of one allocation if boxing occurs.

@danielcweber
Copy link
Collaborator Author

So what's your proposal for a trampoline with state? Duplicate AsyncLock with a generic type parameter?

@akarnokd
Copy link
Collaborator

akarnokd commented Jun 1, 2018

Yes, don't penalize the stateless user places.

@danielcweber
Copy link
Collaborator Author

That would work if all places in a class would only ever pass the same type of state. This might be the case in Rx but it would severely limit the usefulness of AsyncLock. Hence the object parameter.

@akarnokd
Copy link
Collaborator

akarnokd commented Jun 1, 2018

Why would it be public? Support Rx primarily, also T can be object.

@danielcweber
Copy link
Collaborator Author

Will revisit after #558 is merged.

@danielcweber
Copy link
Collaborator Author

@akarnokd Revised the approach. Boxing will still occur when TState is a value type, however, it's still an improvement over closure+delegate.

@danielcweber danielcweber merged commit f4f6a62 into dotnet:master Jun 6, 2018
@danielcweber danielcweber deleted the AsyncLockWithStateParameter branch June 6, 2018 20:08
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