Optimize field access in types not marked with beforefieldinit #1327
Description
Is it possible for tiered compilation to partially alleviate my issue regarding beforefieldinit by addressing the performance aspect of that issue discussed here: dotnet/csharplang#3080?
I would still like the option of explicit control over class initialization timing as discussed there but it would be much less of an issue in most cases if tiered compilation could at least solve the performance aspects. For reference, here is a copy-paste of the benchmark code in that thread that shows the big performance discrepancy between static field access in classes with and without beforefieldinit:
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;
}
}category:cq
theme:optimization
skill-level:intermediate
cost:medium
Metadata
Metadata
Assignees
Type
Projects
- Status
Activity
AndyAyersMS commentedon Jan 6, 2020
Thanks; yes this
is something we can fixa consequence of us not re-jitting methods with loops, by default (see note below).The difference right now is that the
Fieldcase hoists the static base accessor out of the loop, and theStaticcase does not:The code in
optComputeLoopSideEffectsOfBlockis too conservative, it checks if the helper call could cause class initialization, but doesn't also check if the class happens to already have been initialized.jkotas commentedon Jan 6, 2020
@AndyAyersMS If the class have been initialized already, the importer won't create
CORINFO_HELP_GETSHARED_NONGCSTATIC_BASEat all.To fix this specific case, we would need to run the static initialization during the first iteration of the loop only.
mikernet commentedon Jan 6, 2020
If this issue is fixed, are there still going to be significant performance penalties for static field access on non-beforefieldinit types in some situations or has that largely been mitigated in newer .NET Core releases? I haven't seen any recent discussions on the performance implications of having a static constructor. The latest thing I could find is this PR dotnet/corefx#6016 where it seems like people are still trying to avoid static constructors where possible for performance reasons.
jkotas commentedon Jan 6, 2020
Depends what you mean by significant. Non-beforefieldinit type are more expensive than beforefieldinit types by design. Note that the costs are not just in throughput, but also in code size.
mikernet commentedon Jan 6, 2020
That's what I thought, but @YairHalberstadt seemed to think that tiered compilation could/should remove the overhead of accessing the field.
jkotas commentedon Jan 6, 2020
Yes, it does. It takes some time for the tiered compilation to kick in and you pay the costs until that happens. I do not think it kicked in for your test at the top of this issue.
mikernet commentedon Jan 6, 2020
Hmm, I see. What's the order of magnitude of "some time" / what conditions affect how long it takes?
I'm trying to gauge if it is still worth complicating class initialization code, removing
readonlyfield modifiers, making it more cumbersome to work with NRT fields, etc in order to avoid having a static constructor in performance-sensitive code like a DB engine. If it takes on the order of seconds for that to kick in and optimize the code then I guess it usually isn't that big of a deal.I mean, ideally, we just get the attribute I proposed in the related issue so the tradeoff becomes unnecessary, but I wouldn't mind refactoring some nasty code if it ultimately doesn't make a difference after a few second warmup.
AndyAyersMS commentedon Jan 6, 2020
Right, just came to the same realization. Assembly code above is for the case where the .cctors have not yet been run. We only jit these methods once: they have a loop, and by default
COMPlus_TC_QuickJitForLoops=0, so they bypass Tier0.Not in this case, though. We jit at Tier1 initially because of the loop, and won't ever rejit. If you set
COMPlus_TC_QuickJitForLoops=1and run, then tiering can remove the class init overhead:YairHalberstadt commentedon Jan 6, 2020
Presumably in real life code if this is a bottleneck we will rejit, since the loop is unlikely to be top level?
mikernet commentedon Jan 6, 2020
Is there any way to force tiered compilation to run before a test starts so that I can benchmark the post-tiered compilation performance impact of adding static initializers to some of my code?
AndyAyersMS commentedon Jan 6, 2020
Not currently, no. Once we jit at Tier1 we never rejit. So with default settings, methods with loops are jitted just once.
The intent of QJFL=0 is to avoid getting trapped in Tier0 code; the downside is you may end up with sub-optimal Tier1 code.
The general fix for this is to implement something like on-stack replacement, so we can jit at Tier0 initially and transition (mid-method, if necessary) to Tier1 code if the method is called frequently, or loops around frequently.
BenchmarkDotNet should generally get you to Tier1 code. But as noted above, not all Tier1 code is equally performant; there's a benefit in some cases to running the Tier0 code first. Setting
COMPlus_TC_QuickJitForLoops=1will ensure this.17 remaining items
EgorBo commentedon Feb 3, 2021
@jkotas @AndyAyersMS a few questions:
Can we just hoist
call CORINFO_HELP_GETSHARED_NONGCSTATIC_BASEfrom the loop body inStaticjust like it was done forField? https://www.diffchecker.com/mTHFbDfu (Staticis on the left)Regarding the inlined "is init" check - if I understand it correctly it should be something like this?
EgorBo commentedon Feb 4, 2021
Quick prototype for "inlined check": EgorBo@78436b0 (jit only, doesn't do hoisting opt)
diff for
Staticmethod: https://www.diffchecker.com/9swAJvSljkotas commentedon Feb 4, 2021
In general case, it would be observable behavior change in the timing of static constructors. You can do this in some limited cases like when you can prove the loop will run at least once and the static is accessed as the first thing in the loop, etc.
The offsets and masks should be abstracted via JIT/EE interface. It will also allow enabling this optimization for non-shared generics.
EgorBo commentedon Apr 27, 2023
This issue is basically fixed by QuickJit enabled by default for Loops in .NET 7.0
For NativeAOT and TieredCompilation=0 it's fixed in .NET 8.0 via #83911
TC=0 .NET 7.0:
TC=0 .NET 8.0: