use ref instead of in for generic T + interface

[bollhals]

Proposed Changes

Figured this out while #1096, as the performance was not what I wanted it to be. While investigating I figured out that when we use method<T>(in T parameter) where T: struct, Interface and then call a method on it, the compiler is unable to figure out that they might be readonly. The effect of it is that it does a defensive copy. (Link to dotnet issue)

While the structs itself are usually quite small, it still quite noticable.

The workaround for this is to use ref instead of in, as this allows calling methods on it without defensive copies.

Types of Changes

• Bug fix (non-breaking change which fixes issue #NNNN)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause an observable behavior change in existing systems)
• Documentation improvements (corrections, new content, etc)
• Cosmetic change (whitespace, formatting, etc)

Checklist

• I have read the CONTRIBUTING.md document
• I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
• All tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)
• Any dependent changes have been merged and published in related repositories

Further Comments

After

Method	When	Runtime	Mean	Error	StdDev	Code Size
BasicPublishWriteNonEmpty	Before	.NET 4.8	375.61 ns	2.775 ns	2.596 ns	6935 B
BasicPublishWriteNonEmpty	After	.NET 4.8	370.54 ns	1.648 ns	1.542 ns	6831 B
BasicPublishWrite	Before	.NET 4.8	206.93 ns	0.450 ns	0.399 ns	6807 B
BasicPublishWrite	After	.NET 4.8	203.22 ns	0.407 ns	0.361 ns	6703 B
BasicPublishMemoryWrite	Before	.NET 4.8	145.62 ns	0.574 ns	0.537 ns	7208 B
BasicPublishMemoryWrite	After	.NET 4.8	138.49 ns	0.663 ns	0.620 ns	7044 B
BasicPublishWriteNonEmpty	Before	.NET Core 3.1	263.46 ns	1.574 ns	1.473 ns	5776 B
BasicPublishWriteNonEmpty	After	.NET Core 3.1	250.61 ns	1.237 ns	1.157 ns	5638 B
BasicPublishWrite	Before	.NET Core 3.1	115.68 ns	0.390 ns	0.364 ns	5776 B
BasicPublishWrite	After	.NET Core 3.1	106.93 ns	0.436 ns	0.408 ns	5638 B
BasicPublishMemoryWrite	Before	.NET Core 3.1	71.74 ns	0.274 ns	0.243 ns	6318 B
BasicPublishMemoryWrite	After	.NET Core 3.1	60.68 ns	0.356 ns	0.333 ns	6127 B

[michaelklishin]

@bollhals this should be merged after #1096, am I right?

[bollhals]

No, this one first, as this is a easy straight forward change. I'll have to tweak the other a bit more, I'm not yet satisfied (that's why I haven't posted any benchmarks yet)

[michaelklishin]

FWIW this fails to backport to 7.x. Maybe we don't want it here, I just decided to try.

[michaelklishin]

Thank you, @bollhals!

[bollhals]

FWIW this fails to backport to 7.x. Maybe we don't want it here, I just decided to try.

Isn't main = 7.x until fhe fist release? Or what gets into 7.x?

[michaelklishin]

Haha, I don't even know any more. I thought main has some 8.x-specific things. But we can ship 7.x off of it, too.

[stebet]

This makes perfect sense since in basically is a message to the compiler saying: "This should be a read-only reference" so in forces a stack copy to be made. Good catch!

From https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/in-parameter-modifier

The in keyword causes arguments to be passed by reference but ensures the argument is not modified. It makes the formal parameter an alias for the argument, which must be a variable. In other words, any operation on the parameter is made on the argument. It is like the ref or out keywords, except that in arguments cannot be modified by the called method. Whereas ref arguments may be modified, out arguments must be modified by the called method, and those modifications are observable in the calling context.

[bollhals]

This makes perfect sense since in basically is a message to the compiler saying: "This should be a read-only reference" so in forces a stack copy to be made. Good catch!

Yes, but the strange thing is even if you have a generic method, it still doesn't recognizes the readyonly of the implementig method, it only seems to look at the interface, for which you can't put readonly on methods...