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 Service] Re-evaluate the choice of serialization technology #5939

Closed
Tracked by #3139
ladipro opened this issue Dec 7, 2020 · 1 comment
Closed
Tracked by #3139
Labels
performance Performance-Scenario-Build This issue affects build performance. triaged
Milestone

Comments

@ladipro
Copy link
Member

ladipro commented Dec 7, 2020

StreamJsonRpc, as originally chosen to use for cross-process RAR request serialization, has proven problematic and ended up being the primary reason for reverting the work. It brings in a bunch of dependent assemblies: StreamJsonRpc, Nerdbank.Streams, MessagePack, MessagePack.Annotations. Surprisingly also Microsoft.VisualStudio.Threading, Microsoft.VisualStudio.Validation because of its close ties with Visual Studio.

  • The dependencies raise eyebrows - Why do I need "VisualStudio" in my .NET SDK package, it's adding size, etc. - and will add burden going forward synchronizing the dependencies with anything else running inside VS.
  • There is a known issue with another VS component which mistakenly loads Microsoft.VisualStudio.Threading from MSBuild directory (it's not uncommon to have this directory on the assembly load path), the proposed solution to which would be to move the assembly to a subdirectory under MSBuild.

Alternative solutions:

  • Stick with the insecure BinaryFormatter for now. Sooner or later MSBuild will have to move over to something else anyways and adding this one use-case shouldn't incur much additional cost.
  • Hand-write a custom serializer (no external dependecies!).
  • Pick another existing serialization library: Protocol buffers, Bond, ...

What should be the criteria on which to pick the protocol?

  • Perf, security, protocol actively maintained, easy to deploy (needs runtime dependencies or not), homogeneity (with MSBuild and other .NET tools), adoption cost, …

Next steps: Define criteria, assign weights, investigate and measure to fill out a table, and come up with the winner.

@ladipro
Copy link
Member Author

ladipro commented Feb 1, 2021

We have decided to keep using the existing serialization technology based on BinaryFormatter and have the parent User Story focused only on RAR and not on this orthogonal problem. BinaryFormatter will be replaced in all of MSBuild IPC later.

@ladipro ladipro closed this as completed Feb 1, 2021
@ladipro ladipro added this to the MSBuild 16.10 milestone Dec 9, 2021
@AR-May AR-May added the triaged label Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance-Scenario-Build This issue affects build performance. triaged
Projects
None yet
Development

No branches or pull requests

2 participants