Skip to content

Update EncodingTable to use ConcurrentDictionary for performance#125001

Merged
stephentoub merged 4 commits intomainfrom
copilot/update-encodingtable-concurrentdictionary
Mar 1, 2026
Merged

Update EncodingTable to use ConcurrentDictionary for performance#125001
stephentoub merged 4 commits intomainfrom
copilot/update-encodingtable-concurrentdictionary

Conversation

Copy link
Contributor

Copilot AI commented Feb 28, 2026

Replace Dictionary + ReaderWriterLockSlim with ConcurrentDictionary in System.Text.Encoding.CodePages's EncodingTable.cs, matching the pattern already used in System.Private.CoreLib's EncodingTable. This eliminates lock acquisition on the fast read path — ConcurrentDictionary.TryGetValue is lock-free.

Description

Changes Made

  • EncodingTable.cs: Replace 4 Dictionary fields + 1 ReaderWriterLockSlim with 4 ConcurrentDictionary fields using target-typed new() expressions. Simplify GetCodePageItem, GetCodePageFromName, and GetNameFromCodePage from nested EnterUpgradeableReadLock/EnterWriteLock patterns to straightforward TryGetValue/TryAdd. All multi-line if blocks use braces consistently.
  • .csproj: Add System.Collections.Concurrent project reference for $(NetCoreAppCurrent) targets.
  • Performance validation: Triggered @EgorBot benchmark comparing PR vs base branch on linux_amd and osx_arm64, exercising hot cache-hit paths through CodePagesEncodingProvider.GetEncoding(name) and encoding property accessors.

Testing

  • ✅ Build succeeds — 0 warnings, 0 errors across all target frameworks
  • ✅ All 1250 existing tests pass with 0 failures
  • @EgorBot performance benchmark triggered (awaiting results)

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits February 28, 2026 04:15
…EncodingTable

Replace the Dictionary<K,V> + ReaderWriterLockSlim pattern with
ConcurrentDictionary<K,V> in System.Text.Encoding.CodePages's
EncodingTable.cs. ConcurrentDictionary provides a lock-free fast
read path via TryGetValue, avoiding the overhead of acquiring a
ReaderWriterLockSlim for every cache lookup.

- s_nameToCodePageCache, s_codePageToWebNameCache,
  s_codePageToEnglishNameCache, s_codePageToItemCache all changed
  to ConcurrentDictionary.
- GetCodePageItem, GetCodePageFromName, GetNameFromCodePage simplified
  to use TryGetValue/TryAdd instead of lock-based patterns.
- Added System.Collections.Concurrent project reference for net11.0+
  targets.

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
@stephentoub
Copy link
Member

@copilot use the performance benchmark skill to validate this change

Copy link
Contributor Author

Copilot AI commented Feb 28, 2026

@copilot use the performance benchmark skill to validate this change

Running benchmark via @EgorBot. The benchmark exercises the internal EncodingTable cache lookup paths (which changed from Dictionary + ReaderWriterLockSlim to ConcurrentDictionary) through the public CodePagesEncodingProvider API — both the cold (first-call, populates cache) and hot (subsequent calls, cache hit) paths.

@EgorBot -linux_amd -osx_arm64

using System.Text;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

BenchmarkSwitcher.FromAssembly(typeof(Bench).Assembly).Run(args);

[MemoryDiagnoser]
public class Bench
{
    // Encoding names that exercise EncodingTable.GetCodePageFromName -> cache lookup
    private static readonly string[] EncodingNames = new[]
    {
        "shift_jis", "big5", "gb2312", "euc-kr", "iso-8859-2",
        "windows-1252", "windows-1251", "windows-1250", "windows-874", "ibm437"
    };

    private Encoding[] _cachedEncodings = default!;
    private EncodingProvider _provider = default!;

    [GlobalSetup]
    public void Setup()
    {
        Encoding.RegisterProvider(CodePagesEncodingProvider.Instance);
        _provider = CodePagesEncodingProvider.Instance;

        // Pre-populate internal caches by resolving each encoding once
        _cachedEncodings = new Encoding[EncodingNames.Length];
        for (int i = 0; i < EncodingNames.Length; i++)
        {
            _cachedEncodings[i] = _provider.GetEncoding(EncodingNames[i])!;
        }
    }

    // Hot path: repeated lookups by name (hits ConcurrentDictionary/Dictionary cache)
    [Benchmark]
    public int GetEncodingByName_Cached()
    {
        int sum = 0;
        for (int i = 0; i < EncodingNames.Length; i++)
        {
            sum += _provider.GetEncoding(EncodingNames[i])!.CodePage;
        }
        return sum;
    }

    // Hot path: access properties that read from name/item caches
    [Benchmark]
    public int AccessEncodingProperties_Cached()
    {
        int sum = 0;
        for (int i = 0; i < _cachedEncodings.Length; i++)
        {
            var enc = _cachedEncodings[i];
            sum += enc.WebName.Length;
            sum += enc.EncodingName.Length;
            sum += enc.WindowsCodePage;
            sum += enc.IsSingleByte ? 1 : 0;
        }
        return sum;
    }

    // Single lookup (hot path, most representative of typical usage)
    [Benchmark]
    public Encoding GetSingleEncoding_Cached()
    {
        return _provider.GetEncoding("shift_jis")!;
    }
}

@stephentoub stephentoub changed the title [WIP] Update EncodingTable to use ConcurrentDictionary for performance Update EncodingTable to use ConcurrentDictionary for performance Feb 28, 2026
@stephentoub stephentoub requested a review from tarekgh February 28, 2026 09:50
@stephentoub stephentoub marked this pull request as ready for review February 28, 2026 09:50
Copilot AI review requested due to automatic review settings February 28, 2026 09:50
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR replaces the locking mechanism in System.Text.Encoding.CodePages's EncodingTable.cs for improved performance. The original code used four Dictionary instances protected by a single ReaderWriterLockSlim, where even reads required acquiring an upgradeable read lock. The replacement uses four ConcurrentDictionary instances, whose TryGetValue is lock-free on the read path (hot path), matching the pattern already established in System.Private.CoreLib's EncodingTable.

Changes:

  • Replace Dictionary<> fields + ReaderWriterLockSlim with ConcurrentDictionary<> fields and simplify all three cache-using methods (GetCodePageItem, GetCodePageFromName, GetNameFromCodePage) from nested lock patterns to straightforward TryGetValue/TryAdd calls.
  • Add a ProjectReference to System.Collections.Concurrent for NetCoreAppCurrent-compatible targets, which is the correct gating for source-build project references.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
EncodingTable.cs Replaces Dictionary + ReaderWriterLockSlim with ConcurrentDictionary; removes ~50 lines of boilerplate locking code
System.Text.Encoding.CodePages.csproj Adds System.Collections.Concurrent project reference for NetCoreAppCurrent-compatible builds

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
@stephentoub stephentoub enabled auto-merge (squash) February 28, 2026 11:45
@tarekgh
Copy link
Member

tarekgh commented Mar 1, 2026

/ba-g the WASM build failure is unrelated to the change and should be already addressed by #125014

@stephentoub stephentoub merged commit 348ad67 into main Mar 1, 2026
85 of 92 checks passed
@stephentoub stephentoub deleted the copilot/update-encodingtable-concurrentdictionary branch March 1, 2026 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants