CallerSymbolDefined: conditional DEBUG across assemblies

[dellamonica]

The well-known Conditional attribute is used to modify the call sites by removing the call if the compilation symbol (e.g. "DEBUG") is not set. This is useful for many scenarios where one typically wants to do additional, possibly expensive checks in Debug without taking the performance hit on release builds.

This however does not work across assemblies.

This proposal suggests extending Caller attributes (see #87) with an attribute that allows the compiler to bake-in a boolean for specific symbols.

public class MyLibrary
{
    public int Foo(Something s, [CallerSymbolDefined("DEBUG")] bool isDebug = false)
    {
         if (isDebug)
         {
              RunExpensiveValidationChecks(s);
         }
         // ...
         return 0;
    }
}

[colejohnson66]

What we ended up doing at my job is have our CI build two copies of the shared library. Assuming this library is called “Xyz”, we’d build one called Xyz (in release mode) and one called Xyz.Debug (in debug mode). Then it uploads both to the private NuGet server. On the application side, we have our csproj conditionally pick which one to reference. So, in debug mode, the csproj will reference Zyz.Debug, but when publishing, it’ll reference Xyz.

It’s a neat hack that works really well. We just sprinkle our library full of Debug.Assert and they’ll still work in the application. Then, when publishing a new build, all those assertions magically disappear.

[dellamonica]

This is a creative solution, but it places a cost on all projects, both library and consumer.

If you are willing to be hacky on the library side, you could have a static LibraryDebug.Enable() method. Then the verification code checks that static bool. However, this is not terribly efficient because the JIT cannot propagate constants, eliminate dead branches, and possibly inline code (which is one of the benefits of the caller attribute I'm proposing).

[theunrepentantgeek]

the JIT cannot propagate constants, eliminate dead branches, and possibly inline code

Tiered JIT compilation does all of those things for hot Code paths.

I remember one case study showing how it could detect when initialisation code was now dead (since that branch had already executed) and could be elided from the fast path code.

[dellamonica]

the JIT cannot propagate constants, eliminate dead branches, and possibly inline code

Tiered JIT compilation does all of those things for hot Code paths.

I remember one case study showing how it could detect when initialisation code was now dead (since that branch had already executed) and could be elided from the fast path code.

You quote replied a sentence and it came out of context. What I said is that by using my suggested attribute you get all these nice optimizations from the JIT as it can see at the call site a constant being passed to the method. Other approaches, such as using a static field are much harder on the JIT.

[theunrepentantgeek]

Quoting [Performance Improvements in .NET 8}(https://devblogs.microsoft.com/dotnet/performance-improvements-in-net-8/#constant-folding):

There are a multitude of these kinds of improvements in .NET 8. dotnet/runtime#77102 made it so that a static readonly value type’s primitive fields can be constant folded as if they were themselves static readonly fields, and dotnet/runtime#80431 extended that to strings. dotnet/runtime#85804 taught the JIT how to handle RuntimeTypeHandle.ToIntPtr(typeof(T).TypeHandle) (which is used in methods like GC.AllocateUninitializedArray), while dotnet/runtime#87101 taught it to handle obj.GetType() (such that if the JIT knows the exact type of an instance obj, it can replace the GetType() invocation with the known answer).

The article is long, and goes into a lot of detail, but it's totally worth reading.

In many cases, the value of a readonly field - especially a static one - can be inlined as a part of tiered recompilation.

[dellamonica]

Quoting [Performance Improvements in .NET 8}(https://devblogs.microsoft.com/dotnet/performance-improvements-in-net-8/#constant-folding):

There are a multitude of these kinds of improvements in .NET 8. dotnet/runtime#77102 made it so that a static readonly value type’s primitive fields can be constant folded as if they were themselves static readonly fields, and dotnet/runtime#80431 extended that to strings. dotnet/runtime#85804 taught the JIT how to handle RuntimeTypeHandle.ToIntPtr(typeof(T).TypeHandle) (which is used in methods like GC.AllocateUninitializedArray), while dotnet/runtime#87101 taught it to handle obj.GetType() (such that if the JIT knows the exact type of an instance obj, it can replace the GetType() invocation with the known answer).

The article is long, and goes into a lot of detail, but it's totally worth reading.

Those are indeed great and very thorough.

In many cases, the value of a readonly field - especially a static one - can be inlined as a part of tiered recompilation.

I don't see how it would apply to an alternative to this proposal in particular. To be clear, my proposal enjoys all the optimization goodies the JIT can provide because the C# compiler would have baked in a constant value for the isDebug argument at the call site. The JIT then sees that the callee has a branch on isDebug, but since it is not taken, inlining the method might become more advantageous (since the inlined code can completely eliminate the branch).

[HaloFour]

This seems quite invasive, in that you'd end up having to plumb this flag through all code that may be interested in it and it would be a permanent part of the API of that library. I think having separate debug/release assemblies is definitely the better way to do, and would put a lot less onus on the runtime to optimize properly as in the latter case there isn't any additional code to elide.

[HaloFour]

but I also consider it "invasive" as the packages are all duplicated and the consumer needs to setup conditionals on the .csproj.

These sound like concerns that could be addressed by tooling.

Otherwise, I would suggest setting a constant to true/false based on the pre-processor directive and passing that to the methods. It's more verbose, but it's also more clear what is going on.

[dellamonica]

@dellamonica To be clear, I do think something like this could be beneficial is certain niche cases, but I don’t think it’s beneficial enough for it to be worth the effort it would place on the LDM.

There is some work on caller attributes already, that's the only reason I think this could be a cheaper feature, it would be a marginal cost on top of the work in other attributes. It is indeed a niche case, but this can be said of most of the other attributes.

What @HaloFour is getting at is: this boolean would need to be plumbed through all API layers for maximum benefit. The team would also need an analyzer to ensure you don’t accidentally forget forwarding it. After all, a symbol is similar to a magical global, so why not just use it as one? My solution of two builds works because it avoids plumbing. It had a relatively large setup to get working, but now that it’s done, things “just work”.

You might be able to do something like this:

public static class Defines
{
#if DEBUG
    public static readonly bool Debug = true;
#else
    public static readonly bool Debug = false;
#endif

Just make the parameter required (not optional), then pass that boolean constant to your APIs.

We actually use something like this at my job as well because we don’t like ifdefs; Syntax checking and refactorings ignore the ifdef’d-out code. So, code we only want to execute on builds meant for our custom RPi device has if (Defines.Pi) DoStuffOnlyOnPi();. The JIT then does constant propagation, so execution time is unaffected.

The plumbing is somewhat annoying, I agree, but I'd say is similar to forwarding cancellation tokens...

There is some discussion on implicits (#5989) which is a feature that could simplify carrying these flags everywhere.

[dellamonica]

but I also consider it "invasive" as the packages are all duplicated and the consumer needs to setup conditionals on the .csproj.

These sound like concerns that could be addressed by tooling.

I'm sure they could, but I have my doubts they will be.

Otherwise, I would suggest setting a constant to true/false based on the pre-processor directive and passing that to the methods. It's more verbose, but it's also more clear what is going on.

This is less discoverable for the consumers of libraries. If you are consuming your own libraries, then all these solutions make perfect sense.

[HaloFour]

I'm sure they could, but I have my doubts they will be.

Changes to tooling are many orders of magnitude easier than changes to the language.

[timcassell]

Is there a simple way to create a single NuGet package that contains the separate assemblies and which "just works" without requiring the consumer project to jump through hoops?

Custom .targets file. Example, it chooses the dll based on the consumer's configuration (Debug or Release), and provides an option to override via an MSBuild property.

The downside to this approach is references aren't transitive by default, unlike with the default nuget mechanism.

[dellamonica]

Is there a simple way to create a single NuGet package that contains the separate assemblies and which "just works" without requiring the consumer project to jump through hoops?

Custom .targets file. Example, it chooses the dll based on the consumer's configuration (Debug or Release), and provides an option to override via an MSBuild property.

The downside to this approach is references aren't transitive by default, unlike with the default nuget mechanism.

Thanks, I didn't know about .targets.