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

RAR-as-a-service draft #11153

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ccastanedaucf
Copy link
Contributor

@ccastanedaucf ccastanedaucf commented Dec 16, 2024

Draft implementation for RAR-as-a-service. This is not intended to be merged, any final implementation would go in as separate small PRs + unit tests.

Known issues

The MSBuild node integration was the last piece implemented, so isn't as thoroughly designed / tested as everything else. I've largely tested this project as a standalone process (the executable produced by RarTest.csproj) to use as a baseline for performance. The node integration is still WIP and I'm moving stuff around, but here are the active items:

  • Right now there's multiple options of command line flag, MSBuild property, and environment variable to enable / disable the RAR server, since I've been testing different approaches. There's a trickiness here since the RAR Task needs to be able to know whether the current build should run out-of-proc, but there doesn't seem to be a very clean way to propagate this.
  • Node exclusivity isn't fully worked out e.g. still playing with using mutex vs just named pipe existence. Some of this is commented out. Needs to work for the quickbuild scenario where MSBuild is invoked per-project, so we need to avoid race conditions.
  • RAR MSBuild node performs worse on first, even second run, compared to manually running RarTest.exe which is functionally the same. Unsure what causes this difference since the node is always done with setup by the time the RAR task is reached. Possibly related to multiple nodes being launched.
  • Passing the RAR flag appears to cause multiple long-lived nodes to be launched from a single /m:1 MSBuild invocation. I suspect these are duplicate RAR nodes, but I haven't figured out what code path is triggering this or an easy way to validate since the processes are all MSBuild.exe.
  • As such, haven't tested MSBuild node implementation on multi-proc.
  • Logging needs to be standardized and doesn't seem to be produced under the Temp\MSBuildTemp\ dir despite using `CommunicationsUtilities.Trace().
  • Passing PipeSecurity to named pipe breaks ability to connect, so temporarily commented out (unknown why this pattern doesn't affect other node implementations)
  • The client and server are similar to BackEnd\Components\Communications\NodeProviderOutOfProcBase and Shared\NodeEndpointOutOfProcBase, so would ideally be derived. Unfortunately, the former has too many dependencies in Microsoft.Build.csproj to easily refactor into Shared. The latter forces the named pipe to have a single server and keep a long-lived connection to the client, and is written around this assumption. This model does not work given multiple MSBuild nodes must send/receive on the pipe at any given time, even with an internal message queue (I've tried).

@ccastanedaucf ccastanedaucf requested a review from a team as a code owner December 16, 2024 18:35
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.

1 participant