microsoft / FASTER

Fast persistent recoverable log and key-value store + cache, in C# and C++.
https://aka.ms/FASTER
MIT License
6.29k stars 563 forks source link

Suggestion: C#: "skip locals init" and "readonly" #904

Open mgravell opened 5 months ago

mgravell commented 5 months ago

I wanted to discuss these before jumping in;

  1. skip locals init
  2. readonly

skip locals init

[module:SkipLocalsInit]`

This changes .locals init to .locals at the IL level; This can make a measurable and worthwhile difference for some scenarios, especially where stackalloc etc is used; for "normal" code, "definite assignment" rules means this is safe; however, there are a few unsafe and stackalloc scenarios where you need to be careful - for example, your fresh stackalloc data will not be zero'd - needs a code review in particular around any stackalloc or "fixed buffer" scenarios.

This is on-by-default in most framework code, and is well understood. Example here


readonly

Marking non-mutating methods on struct types as readonly (when the struct is not itself readonly) has significant advantages for code that uses in or ref readonly semantics (which might be inside your library, or in consumer code) - avoiding a defensive copy of the struct. This should be fully automated via IDE0251 - you should be safe to blindly accept IDE0251 project-wide; for example, for SpanByte it should be fine for Pointer, ToPointer(), Serialized, MetadataSize, Length get, TotalSize, LengthWithoutMetadata, Invalid get, ToPointerWithMetadata, ExtraMetadata get, AsSpan(), AsReadOnlySpan(), AsSpanWithMetadata(), AsReadOnlySpanWithMetadata(), Deserialize(), ToMemoryOwner(), ToByteArray(), ToString(), CopyTo(), TryCopyTo() - and possiibly a few others!

in some cases you might need to cheat, for example changing Unsafe.AsPointer(ref payload) to Unsafe.AsPointer(ref Unsafe.AsRef(in payload)) (which is JITted away to nothing, but makes the compiler happy to readonly)

Happy to contribute some time towards this if they're desirable