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

Use IValueTaskSource in PipeStream on Windows #52695

Merged
merged 2 commits into from
Aug 12, 2021

Conversation

manandre
Copy link
Contributor

Use IValueTaskSource in NamedPipeServerStream on Windows
Remove PipeCompletionSource

Closes #51463

/cc @stephentoub

@ghost
Copy link

ghost commented May 13, 2021

Tagging subscribers to this area: @carlossanlop
See info in area-owners.md if you want to be subscribed.

Issue Details

Use IValueTaskSource in NamedPipeServerStream on Windows
Remove PipeCompletionSource

Closes #51463

/cc @stephentoub

Author: manandre
Assignees: -
Labels:

area-System.IO

Milestone: -

@carlossanlop carlossanlop modified the milestones: Future, 6.0.0 May 14, 2021
@manandre manandre force-pushed the pipestream-ivaluetasksource branch 3 times, most recently from a98fa7c to 3ab26fb Compare June 3, 2021 10:38
@terrajobst terrajobst added the community-contribution Indicates that the PR has been added by a community member label Jul 19, 2021
@jeffhandley
Copy link
Member

@stephentoub This PR is assigned to you for follow-up/decision before the RC1 snap.

@stephentoub
Copy link
Member

I pushed a commit to revise the implementation to match the latest in FileStream.

As for the perf tests, the tests themselves aren't great. I've revised them in dotnet/performance#1897. The relevant ones look fine:

Type Method Job Toolchain size Options Mean Error StdDev Median Min Max Ratio RatioSD Gen 0 Gen 1 Gen 2 Allocated
Perf_NamedPipeStream ReadWrite Job-JQMLDS \main\CoreRun.exe 1000000 Asynchronous 135.7 us 4.70 us 5.41 us 135.8 us 125.9 us 147.4 us 1.00 0.00 - - - 707 B
Perf_NamedPipeStream ReadWrite Job-ZLUHJI \pr\CoreRun.exe 1000000 Asynchronous 137.0 us 1.03 us 0.91 us 137.0 us 135.5 us 138.5 us 1.00 0.04 - - - 336 B
Perf_NamedPipeStream ReadWriteAsync Job-JQMLDS \main\CoreRun.exe 1000000 Asynchronous 137.3 us 0.98 us 0.86 us 137.0 us 136.3 us 139.4 us 1.00 0.00 - - - 641 B
Perf_NamedPipeStream ReadWriteAsync Job-ZLUHJI \pr\CoreRun.exe 1000000 Asynchronous 134.7 us 1.01 us 0.95 us 134.7 us 132.7 us 136.4 us 0.98 0.01 - - - 1 B

@stephentoub
Copy link
Member

@adamsitnik, can you review? Thanks.

@stephentoub
Copy link
Member

There's a real test failure happening here, and I'm trying to figure out why. So far it's only occurred on x86 on Windows 7.

@stephentoub stephentoub merged commit e4b4666 into dotnet:main Aug 12, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.IO community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use IValueTaskSource in PipeStream on Windows
6 participants