Refactor SQLDateTime constants

[xtqqczze]

• Rename constants to use PascalCase instead of prefixed s_ names.
• Use constants instead of static readonly fields
• Use properties instead of static readonly fields for computed values.

Diffs

[dotnet-policy-service]

Tagging subscribers to this area: @roji, @SamMonoRT
See info in area-owners.md if you want to be subscribed.

[xtqqczze]

@MihuBot

[roji]

/cc @cheenamalhotra @David-Engel

[David-Engel]

For those of us who don't make frequent changes to this repo, can you provide a more detailed description of the reason for the changes? Are you just applying coding standards? Are there micro perf improvements in the changes? AI is great at generating the "sometimes obvious" that would help those unfamiliar with the code understand the reason for the changes.

CC: @cheenamalhotra @paulmedynski

[tannergooding]

CC. @jkotas for input on the changes here.

I did explain a bit on some of the nuance that can exist for static SomeStruct Prop => new SomeStruct(cns, cns, cns) vs static readonly SomeStruct Field = new SomeStruct(cns, cns, cns);, but I don't think we really have formal "guidance" on which is preferred for such cases.

For new APIs, we tend to prefer properties always and may use either => new SomeStruct(cns, cns, cns) or => s_backingField so that we're more free to change things in the future. Most of the APIs being touched here, however, are internal or existing APIs and so that doesn't really apply.

This is likely not particularly perf sensitive code, so I expect it probably won't make a meaningful difference in most workloads in practice. I think it might be good to just leave as is "unless" we have perf numbers showing real world benefit -or- we want to push towards standardizing on a particular pattern due to it being better for all our possible compilers (AOT, R2R, JIT, interpreter, etc)

[David-Engel]

This is likely not particularly perf sensitive code, so I expect it probably won't make a meaningful difference in most workloads in practice. I think it might be good to just leave as is "unless" we have perf numbers showing real world benefit -or- we want to push towards standardizing on a particular pattern due to it being better for all our possible compilers (AOT, R2R, JIT, interpreter, etc)

From a SqlClient perspective, this is definitely perf sensitive code. It's hit for every value when inserting or processing database results.

[jkotas]

@jkotas for input on the changes here.

These changes are a micro-optimization. The proposed change is a bit friendlier for AOT and it is saving a bit of JITed code size (100s bytes according to MihuBot/runtime-utils#1599 linked above), at the cost of some code duplication. I do not expect the effect of these changes to be really observable.

[jkotas]

I don't think we really have formal "guidance" on which is preferred for such cases.

.NET Framework design guidelines book has some guidance around this:

• DO use constant fields for constants that will never change.
• When a type has a significant number of predefined instances, but few are used in any application, you may prefer to use get-only properties. Because the .NET Runtime will initialize all of the static fields at the same time, a type with many predefined instances can suffer from visible start-up performance.