Allow explicit control of beforefieldinit (especially with NRTs)

[mikernet]

As a library developer I'm constantly mangling code that would otherwise be much simpler and easier to understand to avoid writing a static constructor that causes the beforefieldinit flag to be removed. Usually, this involves adding a superfluous static bool _initialized = Initialize() field or returning one of the field values with the Initialize() method and setting the rest of the fields inside it. This means my read-only fields can't be marked readonly, and has now become more of a burden with NRTs because now I have to make reference fields all nullable or silence the warning by setting them to default!.

Can we please add a [BeforeFieldInit(bool)] attribute that controls static initialization behavior?

This is a long-standing issue that I've seen discussed countless times in other places (StackOverflow questions, forum posts, team meetings, library PRs, also just found this one in Roslyn: dotnet/roslyn#4448) with a very simple fix which will make NRTs much more pleasant. I think that makes it a good candidate for immediate implementation. It will also remove the need to have empty static constructors which often get mistaken for something that can be removed with no undesirable effects.

[YairHalberstadt]

Why do you need to avoid beforefieldinit from being removed?

[mikernet]

Performance

[YairHalberstadt]

Do you have some measurements of the performance impact of beforeFieldsInit?

[mikernet]

The performance impact of adding a static constructor is well understood and has been measured many times before. Offhand I don't have the numbers, but it adds overhead to every single static field access on the type which can add up.

[mikernet]

See the last 2 posts in this thread, starting with this one: dotnet/roslyn#4448 (comment)

[mikernet]

The "avoid static constructor" issue comes up in lots of libraries. The presence or absence of a static constructor vs field initializers is a very odd way to forcibly determine when the type is initialized but that ship has sailed, so there should at least be a mechanism available to override the default behavior to get what you want.

[mikernet]

These tests measure a 30% difference: http://nickberardi.com/static-constructors-in-net-3-5-still-a-bad-thing/

This guy experienced a much larger difference: https://stackoverflow.com/questions/14570846/why-does-adding-beforefieldinit-drasticly-improve-the-execution-speed-of-generic

These are old posts though, and maybe I'm micro-benchmarking incorrectly because the simple test I just rigged up isn't showing a difference...so it might be the case that "non-beforefieldinit" field access has been optimized recently, but I can't find any discussions about that. If that's the case, then I can't see why beforefieldinit would ever be desirable anymore though, and the attribute would still be nice so that I don't have to litter static constructors everywhere, haha.

Does anyone have more insight into this?

[mikernet]

Nevermind, I'm seeing a massive difference in my test now.

[YairHalberstadt]

Tiered compilation (which is enabled by default in more recent .Net Core runtimes) should be able to eliminate the overhead as when it rejits it knows the static constructor has been called. If this isn't happening maybe you should raise an issue on dotnet/runtime.

[mikernet]

The following consistently shows an 18x performance penalty for accessing the static fields in the class with the static constructor:

public static class StaticCctor
{
	public static bool Value { get; set; }
	
	static StaticCctor() { 
		Value = true;
	}
}

public static class FieldInit
{
	public static bool Value { get; set; } = true;
}

void Main()
{
	int iterations = 100000000;
	bool result;
	
	for (int i = 0; i < 5; i++) {
		RunTest("field init", () => {
			bool value = false;
			for (int i = 0; i < iterations; i++) {
				value = FieldInit.Value;
			}
		});
	}
	
	
	for (int i = 0; i < 5; i++) {
		RunTest("static cctor", () => {
			bool value = false;
			for (int i = 0; i < iterations; i++) {
				value = StaticCctor.Value;
			}
		});
	}
}

public static void RunTest(string name, Action action)
{
	try {
		var sw = new Stopwatch();
		action();
		
		sw.Start();
		
		action();
			
		sw.Stop();
		
		Console.WriteLine($"{name.PadLeft(20)}: {sw.Elapsed}");
	}
	catch (Exception ex) {
		Console.WriteLine($"{name.PadLeft(20)}: {ex.Message}");
	}
}

[YairHalberstadt]

@mikernet

I would suggest using BenchmarkDotNet for performance testing. IMO a homegrown testing framework is practically worthless.

[mikernet]

Here ya go:

BenchmarkDotNet=v0.12.0, OS=Windows 10.0.18362
Intel Core i7-3770 CPU 3.40GHz (Ivy Bridge), 1 CPU, 8 logical and 4 physical cores
.NET Core SDK=3.1.100
  [Host]     : .NET Core 3.1.0 (CoreCLR 4.700.19.56402, CoreFX 4.700.19.56404), X64 RyuJIT
  DefaultJob : .NET Core 3.1.0 (CoreCLR 4.700.19.56402, CoreFX 4.700.19.56404), X64 RyuJIT

| Method |      Mean |     Error |    StdDev |
|------- |----------:|----------:|----------:|
| Static | 18.870 us | 0.1066 us | 0.0997 us |
|  Field |  2.713 us | 0.0099 us | 0.0092 us |

Code:

const int iterations = 10000;

public static class StaticCctor
{
    public static bool Value { get; set; }

    static StaticCctor()
    {
        Value = true;
    }
}

public static class FieldInit
{
    public static bool Value { get; set; } = true;
}

public class BeforeFieldInit
{
    [Benchmark]
    public void Static()
    {
        bool value;

        for (int i = 0; i < iterations; i++)
            value = StaticCctor.Value;

    }

    [Benchmark]
    public void Field()
    {
        bool value;

        for (int i = 0; i < iterations; i++)
            value = FieldInit.Value;
    }
}

[YairHalberstadt]

That does look quite significant (of course the chances are it will be minor in most normal code, as it will usually be dwarfed by other factors, but I can imagine there are some cases where it is actually a bottleneck).

I would again suggest bringing this up with dotnet/runtime, as if tiered compilation can solve this, that would have the benefits of working automatically for all code without needing to manually add the attribute.

[mikernet]

Unfortunately (or fortunately? I do love it immensely, haha) I almost exclusively work on highly performance-sensitive library code like core database engine logic, entity object frameworks, etc.

[mikernet]

It's not about being a "bottleneck" so much as even a 5% gain for a set of optimizations in the type of library code I work on is often a huge win.

[mikernet]

Performance issues aside, controlling the timing of when you want a class to run its static initialization shouldn't be controlled entirely by the presence or absence of a static constructor, that's just so odd...

(I don't think it should be controlled at all by the presence or absence of a static constructor but hey, we are where we are)

[jmarolf]

We should talk to @stephentoub and others working on corefx and get their opinion. If anyone has this as a pain point it is them.

[YairHalberstadt]

There's been some extremely helpful input from @AndyAyersMS and @jkotas over at dotnet/runtime#1327.

If I may summarize the discussion there:

1. In general tiered compilation can mostly eliminate the overhead from removing beforefieldsinit.
2. In some cases the tier 1 Jit happens too early, before the static constructor is called.
3. If this proves to be a pain point, there are a couple of techniques that could be used to solve this in the Jit.

I think this implies that there's unlikely to be a need for this language feature from a performance perspective. Instead, it would be useful to gather evidence that this is an issue that affects real world code, not just benchmarks. That would provide an incentive to optimize this in the Jit.

There may of course still be a desire for this attribute for reasons of semantics rather than performance. However I think that is a rather niche case, and unlikely to be implemented as a language feature.

Instead something like https://github.com/alexb5dh/BeforeFieldInit.Fody might be more appropriate for those who do want to be able to configure this.

[mikernet]

@YairHalberstadt Great to know about the Fody add-in, thanks for pointing that out! I'll probably end using it for now, but IL weavers come with their own set of issues that would be nice to avoid which I'm just learning about now after I thought a Fody plugin would seamlessly solve automating of argument null testing, haha. Turns out there are definitely downsides to using weavers, though I'm not 100% sure to what degree that would apply to this particular use case.

[mikernet]

Still curious what @stephentoub thinks of this as he's been trying to eliminate static constructors in the BCL and I'm sure this attribute could be liberally applied in many other places in the BCL to continue those efforts without having to refactor all the static constructors.

[stephentoub]

I currently personally don't think adding such a control would be worth it.

[mikernet]

Fair enough then, I'll close the issue. Thanks for your input.