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

Named pipe connection code needs to not busy wait #10413

Closed
jaredpar opened this issue Apr 7, 2016 · 8 comments
Closed

Named pipe connection code needs to not busy wait #10413

jaredpar opened this issue Apr 7, 2016 · 8 comments
Assignees
Labels
Area-Compilers Bug Concept-CoreCLR The issue involves operations and features specific to CoreCLR work.
Milestone

Comments

@jaredpar
Copy link
Member

jaredpar commented Apr 7, 2016

The API NamedPipeClientStream::Connect has a busy wait algorithm with no back off semantics. This can create a huge lag in our standard server compilation scenario on a single core machine:

  1. csc spins up a compiler server instance.
  2. Moves to NamedPipeClientStream::Connect.
  3. Server process begins starting up.

Unfortunately on a single core machine the Connect loop can dominate and cause the server to take ~14 seconds to actually start up and process the connection. Need to call Connect with a small timeout here and do a SpinWait to make it less busy.

@jaredpar jaredpar self-assigned this Apr 7, 2016
@jaredpar jaredpar added this to the 1.3 milestone Apr 7, 2016
@agocke
Copy link
Member

agocke commented Apr 7, 2016

Perhaps we should also consider a reflection call to the async API if it's available? When we kick 4.5 support to the curb we can then just always use the async API.

We'll still need the SpinWait for people really running on .NET 4.5, but this may be a bit more future-proof.

@shmao
Copy link

shmao commented Apr 8, 2016

NamedPipeClientStream.ConnectAsync probably wouldn't help. The implementation of the Async API is quite similar to the Sync one. I think it might have the same issue.

@davidfowl
Copy link
Member

/cc @stephentoub

@robertmclaws
Copy link

Hey, all! Just wanted to pop in with a little background info. This is related to an issue running Roslyn in ASP.NET on Azure Websites for C# 6.0 features. Had to uninstall the package from production because that 14 second spin-up was affecting our users. Someone else experienced the same issue and posted about it on StackOverflow.

Thanks for your help, and looking forward to a fix! :)

@stephentoub
Copy link
Member

Need to call Connect with a small timeout here and do a SpinWait to make it less busy.

Note that NamedPipeClientStream in corefx already uses SpinWait:
https://github.com/dotnet/corefx/blob/master/src/System.IO.Pipes/src/System/IO/Pipes/NamedPipeClientStream.cs#L172
which uses a mixture of Thread.SpinWait, Thread.Yield, and Thread.Sleep to avoid these kinds of issues. That fix has not yet been ported back to the .NET Framework, however:
http://referencesource.microsoft.com/#System.Core/System/IO/Pipes/Pipe.cs,1192
I would expect this Roslyn issue to manifest on the .NET Framework but not on .NET Core... if that's not the case, please let us know.

cc: @ianhays

@davidfowl
Copy link
Member

I would expect this Roslyn issue to manifest on the .NET Framework but not on .NET Core... if that's not the case, please let us know.

It's on .NET Framework, not .NET Core.

@jaredpar
Copy link
Member Author

Indeed it's on .NET Framework. Even if we ported the fix there it wouldn't help because this particular scenario, Roslyn + legacy ASP.Net pages, must work on .NET 4.5.

@jaredpar jaredpar added the Concept-CoreCLR The issue involves operations and features specific to CoreCLR work. label May 24, 2016
@jaredpar jaredpar removed their assignment Jun 2, 2016
agocke added a commit to agocke/roslyn that referenced this issue Jun 2, 2016
On .NET 4.5 the implementation for NamedPipeClientStream busy waits
during the timeout period, blocking the running CPU. Fixes dotnet#10413.
agocke added a commit that referenced this issue Jun 3, 2016
On .NET 4.5 the implementation for NamedPipeClientStream busy waits
during the timeout period, blocking the running CPU. Fixes #10413.
@agocke agocke self-assigned this Jun 6, 2016
@agocke
Copy link
Member

agocke commented Jun 6, 2016

Closed by #11709

@agocke agocke closed this as completed Jun 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Bug Concept-CoreCLR The issue involves operations and features specific to CoreCLR work.
Projects
None yet
Development

No branches or pull requests

6 participants