Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating to .net 6 causes higher lock contention #108057

Closed
AlanLiu90 opened this issue Sep 20, 2024 · 2 comments · Fixed by #108135
Closed

Updating to .net 6 causes higher lock contention #108057

AlanLiu90 opened this issue Sep 20, 2024 · 2 comments · Fixed by #108135
Labels
area-System.Threading in-pr There is an active PR which will close this issue when it is merged tenet-performance Performance related issue

Comments

@AlanLiu90
Copy link
Contributor

Description

Recently, I upgraded our game server project to .net 6 from .net core 3.1 and observed Monitor.LockContentionCount became significantly higher during stress test. Using dotnet-trace, I found it's from Thread.SetThreadPoolWorkerThreadName

image-20240920114038998
Looking into the source code, it seems the contention is from the following code:

private static void CreateWorkerThread()
{
    // Thread pool threads must start in the default execution context without transferring the context, so
    // using UnsafeStart() instead of Start()
    Thread workerThread = new Thread(s_workerThreadStart);
    workerThread.IsThreadPoolThread = true;
    workerThread.IsBackground = true;
    // thread name will be set in thread proc
    workerThread.UnsafeStart();
}

private unsafe void StartCore()
{
    lock (this)
    {
        fixed (char* pThreadName = _name)
        {
            StartInternal(GetNativeHandle(), _startHelper?._maxStackSize ?? 0, _priority, pThreadName);
        }
    }
}

private static void WorkerThreadStart()
{
    Thread.CurrentThread.SetThreadPoolWorkerThreadName();

    // ...
}

internal void SetThreadPoolWorkerThreadName()
{
    Debug.Assert(this == CurrentThread);
    Debug.Assert(IsThreadPoolThread);

    lock (this)
    {
        _name = ThreadPool.WorkerThreadName;
        ThreadNameChanged(ThreadPool.WorkerThreadName);
    }
}

Although I haven't seen performance problem yet, but I think it causes some confusion when updating to .net 6 or later versions.

It looks like using SetThreadPoolWorkerThreadName instead of setting Name property is to avoid setting _mayNeedResetForThreadPool. Is it better to modifiy the implementation to avoid this lock contention?

Reproduction

  1. dotnet new console
  2. Modify TargetFramework to net6.0 (Also reproducible in net8.0)
  3. Paste the following code to Program.cs
using System;
using System.Linq;
using System.Threading;

namespace ConsoleApp1
{
    internal class Program
    {
        private static double[] mValues;

        static void Main(string[] args)
        {
            StartBusyThreads();

            long oldCount = Monitor.LockContentionCount;

            ManualResetEvent done = new ManualResetEvent(initialState: false);

            const int MaxCount = 1000;
            int count = 0;

            for (int i = 0; i < MaxCount; i++)
            {
                ThreadPool.QueueUserWorkItem(m =>
                {
                    Thread.Sleep(1000);

                    if (Interlocked.Increment(ref count) == MaxCount)
                    {
                        done.Set();
                    }
                });
            }

            done.WaitOne(TimeSpan.FromSeconds(10));

            Console.WriteLine("LockContentionCount: {0}", Monitor.LockContentionCount - oldCount);
            Console.WriteLine("DummyValue: {0}", mValues.Sum());

            Environment.Exit(0);
        }

        static void StartBusyThreads()
        {
            var values = mValues = new double[Environment.ProcessorCount];
            var threads = new Thread[Environment.ProcessorCount];

            using var sem = new Semaphore(0, Environment.ProcessorCount);

            for (int i = 0; i < Environment.ProcessorCount; i++)
            {
                int index = i;

                var t = new Thread(_ =>
                {
                    sem.Release();

                    while (true)
                    {
                        const int Count = 10000;

                        double t = 0;

                        for (int j = 0; j < Count; j++)
                            t += Math.Sin(j / (double)Count) + Math.Cos(j / (double)Count);

                        values[index] += t;
                    }
                });

                t.Start();
                threads[i] = t;
            }

            for (int i = 0; i < Environment.ProcessorCount; i++)
            {
                sem.WaitOne();
            }
        }
    }
}
  1. build
  2. Use dotnet-trace to run the application:
dotnet-trace collect --clrevents contention+stack --clreventlevel informational --duration 00:00:10 -- ConsoleApp1.exe
@AlanLiu90 AlanLiu90 added the tenet-performance Performance related issue label Sep 20, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Sep 20, 2024
@martincostello
Copy link
Member

Do you see the same with .NET 8? .NET 6 goes out of support in two months.

@AlanLiu90
Copy link
Contributor Author

AlanLiu90 commented Sep 20, 2024

Do you see the same with .NET 8? .NET 6 goes out of support in two months.

Yes, .NET 8 has the same issue.

AlanLiu90 added a commit to AlanLiu90/runtime that referenced this issue Sep 23, 2024
For a worker thread, Thread.StartCore and Thread.SetThreadPoolWorkerThreadName have lock contention.

Fix dotnet#108057
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Sep 23, 2024
@kouvel kouvel closed this as completed in 65061ad Sep 23, 2024
@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Sep 23, 2024
sirntar pushed a commit to sirntar/runtime that referenced this issue Sep 30, 2024
…08135)

For a worker thread, Thread.StartCore and Thread.SetThreadPoolWorkerThreadName have lock contention.

Fix dotnet#108057
@github-actions github-actions bot locked and limited conversation to collaborators Oct 24, 2024
mikelle-rogers pushed a commit to mikelle-rogers/runtime that referenced this issue Dec 10, 2024
…08135)

For a worker thread, Thread.StartCore and Thread.SetThreadPoolWorkerThreadName have lock contention.

Fix dotnet#108057
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Threading in-pr There is an active PR which will close this issue when it is merged tenet-performance Performance related issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants