Brightspace / rules_csharp

Bazel rules for C#
Apache License 2.0
8 stars 5 forks source link

Improve build cache by generating two reference-only DLLs #150

Closed j3parker closed 3 years ago

j3parker commented 3 years ago

Right now we generate two DLLs:

The reference-only DLL is used by dependees. This DLL doesn't change if you change the implementations of methods/etc. or private interfaces, just the public interface.

The catch is that the public interface (more general than the public modifier) includes the internal interface if you make any use of InternalsVisibleTo. This is very common (at least at D2L) to support tests in an external DLL that test internal things.

We could improve cachability if we took an extra attribute in the csharp_* rules, e.g. internals_visible_to = ["A", "B"], and generated a .cs file with contents like:

using System.Runtime.CompilerServices;
[assembly: InternalsVisibleTo( "A" )]
[assembly: InternalsVisibleTo( "B" )]

We would include this .cs file when we generate the two main DLLs. Let's call the reference DLL that comes out of this the "internals-included" reference DLL.

If we then did a second pass without that .cs file and use the -refonly option to generate a third DLL that only includes the public interface. Let's call this the "public-only" reference DLL.

When compiling X which depends on this assembly, if X is A/B then we use the "internals-included" reference DLL and suffer less cachability. But if X is not A/B (the common case!) then we can use the "public-only" reference DLL and get the really good caching.

Our rules could do this logic easily behind the scenes.

What if the user still uses InternalsVisibleTo manually? Then the "public-only" reference DLL would actually be the same (technically not, because it would have extra assembly attributes, but that's not the important thing) and we would have wasted time doing the second pass. This isn't good, but it's fail-safe which is important.

j3parker commented 3 years ago

Note to self about implementation steps (will slowly pick away at this in spare time, although if anyone else wants to pick off parts LMK):