Hashing: avoid intermediate allocations in Md5Hasher and fix broken ThreadLocal usage under NETSTANDARD2_0
- 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
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