Consider testing command lines to identify worker nodes

[rainersigwald]

See #3106 (comment)

Currently, in .NET Core, MSBuild will find all processes running in the dotnet host and attempt to connect to them as though they were MSBuilds. That shouldn't interfere with the other processes unless they have a named pipe named MSBuild{pid}, but it takes time during the node-connection process.

We should explore narrowing down the candidate processes beforehand. We could for example examine process command lines for /nodereuse:true and/or MSBuild.dll.

Questions that need to be answered:

• How can this be done at all? AFAIK, there's no unified .NET API to get command lines of other processes on all platforms, so it might have to be platform-specific.
• Is this faster than the current approach? Examining command lines will take some time.

[cdmihai]

In terms of speed, writing to the pipe means IO and reading the commandline might mean some os calls, if it's not cached in .net.

[rokonec]

Since nodes creating is controlled by main node and nodes have incremental ID. We could create pipe name based on node HostContext data and node id and test existence of node by that named pipe. I did it similarly in my RAR service prototype.
Also having deterministic unique node string id would allowed us to test for named mutex which would be probably faster than named pipe probe.

[rokonec]

During implementation of this I would like to also address perf issue of GetCurrentProcess().SessionId which took up to 10ms on both sides of connection, delaying node creation by 20 ms.

[rainersigwald]

Another thing we can consider is spawning worker nodes as msbuild.exe even on .NET 5, instead of as dotnet.exe path\to\msbuild.dll. That would require sdk/installer work but would be more understandable in Task Manager/ps. But it wouldn't help here if we adopted your new name scheme.

We could create pipe name based on node HostContext data and node id and test existence of node by that named pipe.

This works great in the connecting-to-idle-node case, but how would we handle the "a build is already running and started nodes but they're all busy and we need to connect to a new set" case? Otherwise this sounds great and I wish I'd thought of it :)

[rokonec]

When you call NamedPipeClientStream.Connect(timeout: 0) it finishes relatively fast. As soon as it finds the other side is not waiting for connection, which they are not because they are working and will start wait for connection after its done.
If above is too slow, because every node have deterministic unique node string id, we can also check named computer wide mutex. Node would lock the mutex when is busy and unlock when is free and we would check if mutex is locked or not. We can have two mutexes one for node existence, other for node busy/free state.

[rainersigwald]

I expect attempting to connect with timeout 0 would be fine; my question was more: if we try to connect to MSBuild_17.0.1_Node3 and it fails, we still want to make and connect to another node. What would the pipe name be?

[rokonec]

We have node id between 1..maxNodes. By scanning all known possible names MSBuild_17.0.1_Node{nodeId} we will know which reusable nodes are running and which are busy. If we need to acquire nodes when another paralel build uses them or they are broken we could have 'infinite' sequential scan:

for (int i=s_lastAcquiredNodeId; ;i++) {
  var nodeUid = $"MSBuild_17.0.1_Node{i}";
  if (isNodeProcessRunning(nodeUid)) 
    if (isNodeFree(nodeUid))
      if(Acquire(nodeUid))
        break;
  else
    if(CreateAndAcquire(nodeUid))
      break;
}
s_lastAcquiredNodeId = i;

Verify.Throw(s_acquiredNodes.Count() <= maxNodes);

[danmoseley]

I would like to also address perf issue of GetCurrentProcess().SessionId

I'm guessing the issue is that GetCurrentProcess() is expensive. This is why eg we added Environment.GetProcessPath to replace getting the process.

If you need it I suppose one option is to write your own code for one or more OS. For Windows it might be faster to get Environment.ProcessId, which just wraps Win32 GetProcessId(), then pass it to Win32 ProcessIdToSessionId()

https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-processidtosessionid

https://github.com/dotnet/runtime/blob/01b7e73cd378145264a7cb7a09365b41ed42b240/src/libraries/System.Diagnostics.Process/tests/Interop.cs#L58

Linux would need something else. Again, no data just thinking aloud.

[rokonec]

@danmoseley GetCurrentProcess().SessionId is part of handshake of MSBuild node named pipe connection, however from the MSBuild code it is not obvious for what we actually need it. I believe we don't need it at all. Roslyn compiler service do not have it in its handshake.

[danmoseley]

It's clearly not for security reasons as the handshake can't be trusted. I am curious whether enumerating processes as we do will even return processes from another session. In fact I'm am not sure whether one can have two concurrent sessions with the same user account.