Make HTTP request methods as const strings

[cpp11nullptr]

Make HTTP request methods as const strings as it should never be changed.

[dnfclas]

All CLA requirements met.

[Tratcher]

Thanks for looking at this, but this behavior was intentional. Using statics allows us to use ReferenceEquals for quick equality checks. Consts get embedded in the calling code and no longer ref-equal.

[analogrelay]

Since there's nothing in the code to make it clear that this was intentional, I filed #12441 to add a comment describing this.

[cpp11nullptr]

Thanks, @anurse.

[analogrelay]

No problem! Without that additional context, your proposal to switch to const was entirely valid! Hopefully this helps clarify things.

[Pilchie]

I thought the runtime did ensure that string literals and constants were interned and ReferenceEqual except for a few corner cases involving things in generic types. Is that no longer the case?

[analogrelay]

That does appear to be the case, I did some rough testing. There may be more complicated stuff at play though. I know @benaadams did some comparison of JIT-generated ASM when he made the change in HeaderNames (#9341) and saw a noticeable performance improvement from switching const to static readonly.

In general, I think we prefer to be specific here rather than rely on runtime behavior that we can't clearly assert.

[Pilchie]

In general, I think we prefer to be specific here rather than rely on runtime behavior that we can't clearly assert.

Definitely agree here.

[benaadams]

I thought the runtime did ensure that string literals and constants were interned and ReferenceEqual except for a few corner cases involving things in generic types.

In the same assembly.

Public consts are compiled directly into the il of the referencing assembly if used in default parameters or attributes, so don't share the reference across assemblies.

The runtime can intern and dedupe across assembly but it doesn't do it aggressively (would cause start-up/jit perf impact) and a cross-assembly string const in a method body also prevents inlining https://github.com/dotnet/coreclr/issues/23969

Behind a static readonly field or method it uses a reference into the referenced assembly; and can inline everything happily and referenceequals is guaranteed to work without having to fallback to a char by char comparision.

[Pilchie]

In general, I think we prefer to be specific here rather than rely on runtime behavior that we can't clearly assert.

Ah, now I remember the issue. Thanks @benaadams! Glad we have a comment there now so people understand why the change was made.