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

[WIP] RAR-as-a-service prototype #3914

Closed

Conversation

ccastanedaucf
Copy link
Contributor

@ccastanedaucf ccastanedaucf commented Nov 8, 2018

Prototype implementation for #3139. So far this results in anywhere from 10-27% reduction in incremental build times on a cached RAR execution, and a 7-8% performance penalty on a full RAR execution.

Still haven't spawned the process from MSBuild, so currently to run this you need to set up a separate project, add a reference to the built Microsoft.Build.Tasks.Core.dll, and write something like this:

using System;

using Microsoft.Build.Tasks.ResolveAssemblyReferences.NamedPipeServer;

namespace ConsoleApp
{
    class Program
    {
        static void Main(string[] args)
        {
            var rarService = new ResolveAssemblyReferenceNamedPipeServer();
            rarService.Start();
        }
    }
}

Here are sample perf numbers. Overall build time can fluctuate independent of RAR, but is still given just to put into perspective the percentage of time RAR takes up in a build.

Non-cached represents either a first build which would result in RAR fully executing for each project. Since caching is on currently a per-project basis, in a build with many projects e.g. WebLarge, RAR will only execute again if inputs or files used by that specific project have changed, while the rest will hit the cache. So times below are either worst-case or best-case, all cache misses or all cache hits, and real-world times would fall in between the two.

Edit: updated with times from latest commit, % change in build times, and now profiled on my desktop

Single-proc

Dotnet Console - Master

Target Performance Summary:
      488 ms  Build

Task Performance Summary:
       82 ms  ResolveAssemblyReference

Note: for small projects, a non-cached build will still run faster due to RAR already being JIT'ed
Dotnet Console - RAR-Service Non-Cached (-6%)

Target Performance Summary:
      457 ms  Build

Task Performance Summary:
       39 ms  ResolveAssemblyReference

Dotnet Console - RAR-Service Cached (-10%)

Target Performance Summary:
      438 ms  Build

Task Performance Summary:
       10 ms  ResolveAssemblyReference

WebLarge - Master

Target Performance Summary:
    14623  ms  Build

Task Performance Summary:
     3985 ms  ResolveAssemblyReference

WebLarge - RAR-Service Non-Cached (+8%)

Target Performance Summary:
    15839 ms  Build

Task Performance Summary:
     4889 ms  ResolveAssemblyReference

WebLarge - RAR-Service Cached (-18%)

Target Performance Summary:
    11997 ms  Build

Task Performance Summary:
     962 ms  ResolveAssemblyReference

Multi-proc

Multi-proc is a bit more difficult to measure since tasks times are cumulative in some form, but there's definitely room for improvement. The current locking mechanism just sticks a lock around all the service cache code, which was the easiest/fastest way to get multi-proc working, so I imagine threads are just spending a significant amount of time waiting.

WebLarge Master

Target Performance Summary:
     5709 ms  Build

Task Performance Summary:
    7207 ms  ResolveAssemblyReference

WebLarge - RAR-Service Non-cached (+7%)

Target Performance Summary:
    6084 ms  Build

Task Performance Summary:
    14393 ms  ResolveAssemblyReference

WebLarge - RAR-Service Cached (-27%)

Target Performance Summary:
     4145 ms  Build

Task Performance Summary:
     1751 ms  ResolveAssemblyReference

Todos

  • Improve concurrency model in ResolveAssemblyReferenceCacheService.cs
  • Testing, testing, testing, which should be fairly straightforward since each part of the service is layered via interfaces
  • Investigation on whether FileSystemWatcher is the appropriate class to use. It can miss events if many fire off in a small period of time and overfill the given buffer, but I haven't looked into exactly how many is "many". It also seems to lock files sometimes, but that might be on my end.
  • Have a real way of spawning the service within the build loop. Currently you have to set up a separate project and start the RAR service independently of MSBuild.
  • Adding a flag within Microsoft.CurrentVersion.Targets to enable/disable RAR running out-of-proc. The property has already been added in RAR, so it's a quick fix.
  • Overhead improvements, there are a few TODOs still scattered about in the code where I believe overhead reduction is possible

Project Structure

Figure it's worth doing this since there's 45 changed/added files. Most of the code is isolated under ResolveAssemblyReferences.

Name Description
ResolveAssemblyReferences/Abstractions Interfaces for Engine and Services for unit testing
ResolveAssemblyReferences/BondTypes Bond schemas
ResolveAssemblyReferences/Domain Request/Response model for service layer
ResolveAssemblyReferences/Engine Implementations of IResolveAssemblyReferenceTask.cs (Tasks.ResolveAssemblyReferences.Task doesn't exactly roll off the tongue)
ResolveAssemblyReferences/NamedPipeClient Client for sending requests from the RAR task
ResolveAssemblyReferences/NamedPipeServer Server for executing requests as a service
ResolveAssemblyReferences/Serialization Bond utilities
ResolveAssemblyReferences/Services Implementations of IResolveAssemblyReferenceService.cs

@ccastanedaucf
Copy link
Contributor Author

ccastanedaucf commented Nov 16, 2018

Looks like a lot of the remaining overhead is due to logging BuildEventArgs. Since RAR has no way of accessing the logging verbosity, these events are always returned in the response object and logged. Because these consist of potentially tens of thousands of strings, this blows up the response payload size and slows down serialization.

Check out the perf difference if I still capture all build events, but simply don't return them in the final response, mocking what would happen if RAR saw a silent verbosity. This brought RAR-as-a-service to a 17-33% reduction in incremental build times on a cached RAR execution, and a 2-6% performance penalty on a full RAR execution:

Single-proc

Dotnet Console - RAR-Service Non-Cached (-12%)

Target Performance Summary:
      428 ms  Build

Task Performance Summary:
        28 ms  ResolveAssemblyReference

Dotnet Console - RAR-Service Cached (-17%)

Target Performance Summary:
      403 ms  Build

Task Performance Summary:
       0 ms  ResolveAssemblyReference

WebLarge - RAR-Service Non-Cached (+6%)

Target Performance Summary:
    15457 ms  Build

Task Performance Summary:
     4444 ms  ResolveAssemblyReference

WebLarge - RAR-Service Cached (-20%)

Target Performance Summary:
    11687 ms  Build

Task Performance Summary:
     644 ms  ResolveAssemblyReference

Multi-proc

WebLarge - RAR-Service Non-cached (+2%)

Target Performance Summary:
    5811 ms  Build

Task Performance Summary:
    13107 ms  ResolveAssemblyReference

WebLarge - RAR-Service Cached (-33%)

Target Performance Summary:
     3823 ms  Build

Task Performance Summary:
     1196 ms  ResolveAssemblyReference

While looking at this, I also found that the reported RAR task time under Task Performance Summary seems like it might be above the actual time . For example, on a single-proc WebLarge run which produced this summary:

Target Performance Summary:
    11772 ms  Build

Task Performance Summary:
     658 ms  ResolveAssemblyReference

the same DotTrace profile shows this when looking at ResolveAssemblyReference.Execute():

image

So either something outside the task method is being counted in the overall time, or the time reported is a bit over the actual time.

@rainersigwald
Copy link
Member

Closing since we haven't been able to land this yet. Hopefully we can pick it up from this point again.

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.

2 participants