dotnet / dotnet/fsharp

Hashing: avoid intermediate allocations in Md5Hasher and fix broken ThreadLocal usage under NETSTANDARD2_0

Open
#20,204 0 comments 0 reactions 0 assignees View on GitHub
Needs-Triage
Dominant language
F#
Stars
4.3k
Forks
876
Avg merge
4d 11h
Merged PRs (30d)
131

Description

Improvements to `Internal.Utilities.Hashing.Md5Hasher` (`src/Compiler/Facilities/Hashing.fs`) to reduce allocations and fix a broken `ThreadLocal` workaround.

**Repro steps**

Not applicable — this is a performance/code-quality improvement, not a functional bug repro. Relevant code path: `Md5Hasher.computeHash`, `Md5Hasher.hashString`, `Md5Hasher.hashStringToString` in `src/Compiler/Facilities/Hashing.fs`, used throughout the compiler for cache-key hashing (e.g. `FSharpProjectSnapshot.fs`, `prim-lexing.fs`).

**Expected behavior**

* `Md5Hasher.computeHash` should use the thread-local `MD5` instance (or `MD5.HashData` on modern TFMs) instead of allocating a brand-new `MD5` instance on every call.
* Hashing a string should avoid allocating an intermediate UTF8-encoded byte array on every call, using a pooled buffer instead.
* A hex-string hash helper should be available without needing to convert through an intermediate `byte array -> BitConverter.ToString` allocation path when only the string result is needed.

**Actual behavior**

* `Md5Hasher.computeHash` previously created a new `MD5` instance on every call via `System.Security.Cryptography.MD5.Create()`, ignoring the existing `ThreadLocal` (`md5`) that was supposed to provide reuse, with a `// TODO: the threadlocal is not working in new VS extension` comment marking the workaround.
* `Md5Hasher.hashString` allocated a full UTF8-encoded `byte array` for the input string via `Encoding.UTF8.GetBytes` before hashing, for every call.
* There was no way to compute the MD5 hash of a string directly into a caller-provided buffer or get a hex string result without allocating both the encoded input bytes and (separately) converting the hash bytes to a display string.

**Proposed fix**

```fsharp
open System.Security.Cryptography

module internal Md5Hasher =

#if NETSTANDARD2_0
let private md5 =
new ThreadLocal<_>(fun () -> MD5.Create())

let computeHash (bytes: byte array) = md5.Value.ComputeHash(bytes)
#else
let computeHash (bytes: byte array) = MD5.HashData(bytes)
#endif

let empty = Array.empty

/// Computes the MD5 hash of a string directly into a caller-allocated 16-byte buffer,
/// avoiding the extra allocation of an intermediate hash-result array.
/// The UTF8 encoding buffer is rented from the shared ArrayPool to avoid allocating
/// a byte array the size of the input string on every call.
let hashStringInto (s: string) (destination: Span) =
let encoding = System.Text.Encoding.UTF8
let maxByteCount = encoding.GetMaxByteCount(s.Length)
let rented = System.Buffers.ArrayPool.Shared.Rent(maxByteCount)

try
let byteCount = encoding.GetBytes(s, 0, s.Length, rented, 0)
#if NETSTANDARD2_0
let hash = md5.Value.ComputeHash(rented, 0, byteCount)
hash.CopyTo(destination)
#else
let mutable bytesWritten = 0
MD5.TryHashData(ReadOnlySpan(rented, 0, byteCount), destination, &bytesWritten) |> ignore
#endif
finally
System.Buffers.ArrayPool.Shared.Return(rented)

/// Computes the MD5 hash of a string and returns it as a hex string (matching the format of
/// `toString`), without allocating an intermediate byte array for the UTF8-encoded input
/// (only the 16-byte hash result is allocated).
let hashStringToString (s: string) =
let bytes = Array.zeroCreate 16
hashStringInto s (Span bytes)
BitConverter.ToString(bytes)
```

`hashString` is reimplemented in terms of `hashStringInto` to keep a single hashing code path:

```fsharp
let hashString (s: string) =
let bytes = Array.zeroCreate 16
hashStringInto s (Span bytes)
bytes
```

**Known workarounds**

None required for correctness — the previous code was functionally correct, just allocation-heavy and not reusing the `ThreadLocal` instance as intended.

**Related information**

* Affected file: `src/Compiler/Facilities/Hashing.fs` (and its signature file `src/Compiler/Facilities/Hashing.fsi`)
* Callers affected (indirectly, no source changes required): `src/Compiler/Facilities/prim-lexing.fs`, `src/Compiler/Service/FSharpProjectSnapshot.fs`
* .NET Runtime kind: affects both `.NET Standard 2.0` (`ThreadLocal` path) and modern TFMs (`MD5.HashData`/`MD5.TryHashData` path)
* Repo: dotnet/fsharp

Contributor guide

Open the contributing guide

Research direction

Start in src/Compiler/Facilities/Hashing.fs and its signature file, reading Md5Hasher.computeHash, hashString, and hashStringToString across the NETSTANDARD2_0 and modern-TFM branches. Check the callers in prim-lexing.fs and FSharpProjectSnapshot.fs, then verify that hashing results remain unchanged while the described intermediate allocations and broken ThreadLocal usage are addressed.

Written by the indexing model from the issue text.

Assessment

Tech stack
fsharp
Domain
compilers, performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.