Enable [SkipLocalsInit]

[mgravell]

Same logic etc as https://github.com/microsoft/FASTER/pull/906

Punting on "server" for now - much more stackalloc usage, requires manual inspection to prove not assuming zero-init

In passing, also fixes some allocs in CreateHexId/DefaultHexId discovered while scanning for stackalloc usage:

• in CreateHexId the original string was upper-case, so we end up allocating 2 string each time, not just one
• in DefaultHexId allocated a new string every time; most uses (all, at current) always pass the default; optimize for that

impact: performance (paper-cuts, no single big win)

[badrishc]

This is a solid idea. You are right that server is a big user of stackalloc. Out of curiosity, did you try adding it and notice tests failing out of the box?

[mjgaux]

This is a solid idea. You are right that server is a big user of stackalloc. Out of curiosity, did you try adding it and notice tests failing out of the box?

No; I simply didn't have the time to look at all the uses and evaluate what the code does.

(this is also @mgravell - my primary account has been touched by MSFT, and now I can't touch projects under "Microsoft" except on my work devices)

[mgravell]

Well that's a special tier of horrible hack that I could have quite happily lived without seeing...

On Tue, 9 Apr 2024, 17:32 Paulus Pärssinen, @.***> wrote:

@.**** commented on this pull request.

In libs/common/Generator.cs https://github.com/microsoft/garnet/pull/243#discussion_r1557966462:

     {

         Span<byte> nodeIdBuffer = stackalloc byte[size / 2];
         RandomNumberGenerator.Fill(nodeIdBuffer);

•        return Convert.ToHexString(nodeIdBuffer).ToLowerInvariant();
  

•        char* charBuffer = stackalloc char[nodeIdBuffer.Length * 2]; // not the same as size (if size is odd)
  

Well, there's a way to do that but it's not worth it here 😆

https://github.com/dotnet/runtime/blob/07648642d345087d344bc8b7b9b3ea7c8793bbea/src/libraries/Common/src/System/HexConverter.cs#L196-L197

— Reply to this email directly, view it on GitHub https://github.com/microsoft/garnet/pull/243#discussion_r1557966462, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEHMGKLZ4PP2XVRA2GVR3Y4QJZLAVCNFSM6AAAAABFY6P73WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSOBZGU3DOMJVGA . You are receiving this because you were mentioned.Message ID: @.***>

[badrishc]

As discussed offline, closing PR for now as we are not seeing end-to-end perf advantages for cache workloads. We can revisit this setting in the future. Thanks!