onepagers for MSBuildServer and RAR caching #11005
Conversation
|
|
||
| ### Risks | ||
|
|
||
| The project was already attempted once, however it was postponed because |
There was a problem hiding this comment.
the risk of teammates being loaned to divisional initiatives threatens completion and delivery of relevant work
There was a problem hiding this comment.
Agree, but this is an implicit risk in every project, so I don't think it's necessary to call that out.
| Getting closer to the possibility of decoupling from Visual Studio. | ||
|
|
||
| ### Stakeholders | ||
|
|
There was a problem hiding this comment.
should we coordinate with other teams such as run time when they would enable this?
| Some communication overhead | ||
|
|
||
| ## Plan | ||
|
|
There was a problem hiding this comment.
thhorough testing should be a priority in the effort.
|
|
||
| Small performance improvement in the short term. Enabling further | ||
| optimizations in the long term. (these improvements are for the Dev Kit | ||
| and inner loop CLI scenarios) |
There was a problem hiding this comment.
Do we have some estimation of the performance gain? Eventually, are there any scenarios that would significantly benefit from the build server?
There was a problem hiding this comment.
Currently the performance gain is in not having to start the MSBuild process over and over again for every build. E.g. the perfomance gain is one quite large process being kept alive, which admittedly is not all that much.
However for the future, we can leverage it to cache some additional stuff more agressively. We can also use it to monitor the folders of the last build and for example re-evaluate in the background - and evaluations are costly.
For the concrete numbers @rainersigwald, can you elaborate please? I have the overall knowledge of the stuff we discussed, but not enough data.
There was a problem hiding this comment.
Yeah I don't have concrete numbers. It's likely not huge for most scenarios, though incremental build of a single project (new-to-.NET scenario) will likely see a solid change.
We should collect numbers as part of this work. IMO unless the numbers show things getting worse, it's worth going forward to
- get this feature to "done", and
- give us a place to hang future background processing.
| optimizations in the long term. (these improvements are for the Dev Kit | ||
| and inner loop CLI scenarios) | ||
|
|
||
| Getting closer to the possibility of decoupling from Visual Studio. |
There was a problem hiding this comment.
Can you elaborate a bit on how would the Build Server help in this decoupling?
There was a problem hiding this comment.
Visual Studio is currently acting almost like a MSBuild server would in some ways. It is a persistent process, that is invoking portions of MSBuild as required. It handles caching, has some file monitoring that it uses for knowing what to rebuild and what to keep and probably some other things I'm not aware of.
This creates a tension since the behavior is almost the same as MSBuild, yet different enough to confuse and sometimes cause issues.
In a perfect world, we would like Visual Studio to instead communicate with MSBuild as a process so that the behavior is the same. Currently, they have no reason to do that. We would like to set up the MSBuild server and then start shifting from "VS uses portions of MSBuild directly" to "VS calls MSBuild server with requests".
@JanKrivanek is this close enough description or did I miss something please?
|
|
||
| Tomas Bartonek, Rainer Sigwald. Successful handover means turning on the | ||
| feature, dogfooding it for long enough to ensure we have reasonable | ||
| certainty that nothing breaks and then rolling it out. |
There was a problem hiding this comment.
Are there any other partners that could dogfood the feature? Specifically, Jared mentioned that we could learn from their experiences with compiler server so perhaps they could help us dogfood the feature?
There was a problem hiding this comment.
Yes, we should go to our close-friend repos like SDK and roslyn to get them to opt in before we opt in for everyone.
|
|
||
| ### Stakeholders | ||
|
|
||
| Tomas Bartonek, Rainer Sigwald. Successful handover means turning on the |
There was a problem hiding this comment.
"MSBuild team" would be probably more descriptive for readers who are not familiar with the team members.
|
|
||
| ### Risks | ||
|
|
||
| The project was already attempted once, however it was postponed because |
There was a problem hiding this comment.
Agree, but this is an implicit risk in every project, so I don't think it's necessary to call that out.
|
|
||
| ### Goals and motivations | ||
|
|
||
| 1ES team wants to isolate their File I/O related to the which is causing |
There was a problem hiding this comment.
I don't understand this sentence, is it missing a part? The scenario needs more clarification. What is I/O isolation in this case?
There was a problem hiding this comment.
That seems likely. I'm sorry, my bad. I was editing it and missed a word. I will take a closer look in the next iteration.
If I understood correctly they have complicated infrastructure, and the current MSBuild caching is pulling files from all nodes at once. This creates a tangled mess of IOs together with their other stuff. They want to separate the caching IO to a separate process so that it is separate from the other File IO related stuff so that it stops polluting their logs.
There was a problem hiding this comment.
@ccastanedaucf that sounds accurate to me, right?
|
|
||
| 1ES team will provide the initial cache implementation. We will review | ||
| their PRs and do the performance evaluations. Handover will be | ||
| successful if nothing breaks and we meet our performance requirements. |
There was a problem hiding this comment.
"we meet our performance requirements" - what are they?
There was a problem hiding this comment.
No regression at the minimum. I mentioned it earlier so I ommited it here. I can remedy.
|
|
||
| ### Impact | ||
|
|
||
| The only impact we’re concerned about is the performance. There will be |
There was a problem hiding this comment.
Who will benefit from this feature? 1ES only or external customers as well?
There was a problem hiding this comment.
This mostly depends on the performance impact. If the performance is neutral, it will be mostly 1ES who will benefit (even though I imagine there can be a customer who would benefit in a similar way to the 1ES)
If the performance is positive, then it could have a broader impact.
Also, one of our discussion items was to transition from multiprocess to multithreaded - if we can pull that one off, this will lose the burden of IPC, further improving the perf. However that is more of a "wishlist/longerm" sort of plan.
| ### Cost | ||
|
|
||
| Week for reviewing the provided PR. Additional two weeks for performance | ||
| testing conditional on the Perfstar infrastructure being functional. |
There was a problem hiding this comment.
Is PerfStar a prerequisite for this then? Could this be done without PerfStar by testing the performance manually?
There was a problem hiding this comment.
We could do it manually, it would be more difficult and costly. Also, we want to have Perfstar for setting the baselines as far as I can tell.
|
|
||
| 1ES team creates the PR wih the RAR cache implementation. | ||
|
|
||
| We review the PR with a special emphasis on the performance side of |
There was a problem hiding this comment.
What happens after the review? What is the expected cost of supporting 1ES? Are we expecting some follow-ups on this?
There was a problem hiding this comment.
Ideally we merge, it works and it's done. There is no expected follow up work besided the usual maintenance.
This should mostly be "only" enabling of a neat feature that will help 1ES and possibly improve performance.
|
Hello @tkapin @donJoseLuis, I went over the comments and answered what I could. |
| that we would communicate with via a thin client. We want to get from | ||
| the current state of “spawn a complete process for every CLI invocation” | ||
| to “we have a server process in the background and we only spawn a small | ||
| CLI handler that will tell the server what to build”. |
There was a problem hiding this comment.
A link to https://github.com/dotnet/msbuild/blob/main/documentation/MSBuild-Server.md is probably also a good idea.
|
|
||
| Small performance improvement in the short term. Enabling further | ||
| optimizations in the long term. (these improvements are for the Dev Kit | ||
| and inner loop CLI scenarios) |
There was a problem hiding this comment.
Yeah I don't have concrete numbers. It's likely not huge for most scenarios, though incremental build of a single project (new-to-.NET scenario) will likely see a solid change.
We should collect numbers as part of this work. IMO unless the numbers show things getting worse, it's worth going forward to
- get this feature to "done", and
- give us a place to hang future background processing.
|
|
||
| Tomas Bartonek, Rainer Sigwald. Successful handover means turning on the | ||
| feature, dogfooding it for long enough to ensure we have reasonable | ||
| certainty that nothing breaks and then rolling it out. |
There was a problem hiding this comment.
Yes, we should go to our close-friend repos like SDK and roslyn to get them to opt in before we opt in for everyone.
|
|
||
| The project was already attempted once, however it was postponed because | ||
| it surfaced a group of bugs that weren’t previously visible due to the | ||
| processes not being persistent. Most of those bugs should be solved by |
There was a problem hiding this comment.
Yeah this is a situation where our change here can reveal latent bugs in other code. We hope that the last attempt flushed out the most critical ones, but I don't know of a way to detect them in advance.
| @@ -0,0 +1,54 @@ | |||
| ## RAR caching | |||
|
|
|||
| During every build we need to gather the graph of references and pass | |||
Onepagers as per our internal team discussion.