ThreadAffinity

[pwkelley]

This is more of a memo than an "issue," because nothing is broken.

I noticed that we're now wrapping any managed -> native operations that require more than one sequential call with Thread.BeginThreadAffinity and Thread.EndThreadAffinity. At least on the Microsoft CLR, these calls are generally unnecessary.

The idea of making managed threads movable from one OS thread to another I believe dates to the development of CLR 2.0 in the early-mid 2000s. The CLR is itself a COM object that can be hosted inside of existing native applications like SQL Server. The CLR is designed to be able to cooperate with a host's requirements around memory management and threading. One thing that SQL Server was designed to do in the 1990s was to be able to use user-mode threads called "fibers" instead of threads. This yields some non-zero performance advantage by avoiding context switches in the kernel. Some significant work was done to allow the CLR 2.0 to be hosted in SQL Server with fibers mode enabled, but I don't think all the kinks were ever worked out, and to my knowledge we never shipped that feature. (The importance of the fiber mode feature in SQL Server has diminished in the past decade.)

Essentially when you call BeginThreadAffinity/EndThreadAffinity, you are calling through the CLR down into the application which is hosting the CLR, and its implementation of IHostTaskManager::BeginThreadAffinity / EndThreadAffinity. With SQL Server in fiber mode, you definitely lose the permanent association of ManagedThreadId with an OS thread, because a fiber can be scheduled by the user-mode scheduler on any OS thread that is participating in that scheduler. In threads mode, I'm not sure. It may be still necessary, but I doubt it.

The default host that you get when you start a managed application directly in Windows doesn't ever move managed threads from one OS thread to another, so these calls do nothing. Some quick Internet searches seem to indicate that Mono has the same behavior.

The calls to Begin/End are technically correct, but there is probably a performance disadvantage, and no correctness advantage in any mainline scenario.

[txdv]

How much overhead are we talking about?
Can you give us some numbers? some profiling numbers?

[nulltoken]

ZOMG! @yorah's going to have a heart attack! 😄

@pwkelley Thanks a lot for this very detailed explanation!

The default host that you get when you start a managed application directly in Windows doesn't ever move managed threads from one OS thread to another, so these calls do nothing

I clearly recall of @xpaulbettsx stating something similar. I haven't been able to find a clear MSDN article about this, though.

• Would that also stand for an IIS host?
• In which cases would calling Thread.BeginThreadAffinity and Thread.EndThreadAffinity make sense?

The default host [...] in any mainline scenario

Could you elaborate a little bit about the "default" or "mainline" aspects? Are there corner cases that should still be considered?

Some quick Internet searches seem to indicate that Mono has the same behavior.

Sorry to ask, but would you please be kind enough to share some pointers on this subject? I'm far from being an expert in this area, and I'd really like to not rely on an implementation detail. Libgit2 aims at being 100% multiplatform, so does LibGit2Sharp and Mono compliancy/compatibility is a must-have feature.

/cc @joncham

Note: More information about the background of this can be found in libgit2/libgit2#624,#203 & #207.

[joncham]

FYI, mono currently binds managed/native threads. The ThreadAffinity methods are currently no-ops on mono. See: https://github.com/mono/mono/blob/master/mcs/class/corlib/System.Threading/Thread.cs#L986

[yorah]

Mmm, I also remember stumbling on the fiber threads thing for SQL Server somewhere on SO (edit: here)
Here are some other pointers I gathered back in the time:

• SO: How often does a managed thread switch os threads
• MSDN

This particular quote from MSDN makes me a bit nervous:

An operating-system ThreadId has no fixed relationship to a managed thread, because an unmanaged host can control the relationship between managed and unmanaged threads. Specifically, a sophisticated host can use the Fiber API to schedule many managed threads against the same operating system thread, or to move a managed thread among different operating system threads.

The Proxy also has some error handling logic in it (e.g.: git_branch_tracking), so we should be careful if we decided to remove it (if we decide to, it doesn't bother me to do it by the way).

Last thought of the day: if in 99% of the use cases we don't need the BeginThreadAffinity/EndThreadAffinity thing, it would definitely make sense to optimize for these 99% use cases in my opinion. If we want to stay on the safe side and support exotic scenarios, one possible option would be to keep the Proxy class, add a compile switch, and modify the DisposableThreadAffinityWrapper private class so that by default, it doesn't call Begin/EndThreadAffinity.

[anaisbetts]

Specifically, a sophisticated host can use the Fiber API to schedule many managed threads against the same operating system thread, or to move a managed thread among different operating system threads.

Sure, but they then can no longer do blocking syscalls, and basically everything libgit2 does is IO (i.e. blocking syscalls). 👍 to nuking the Begin/EndThreadAffinity.

[anaisbetts]

The only sane scenario that I can think of where libgit2sharp could possibly ever need Begin/EndThreadAffinity, is if someone were to try to integrate libgit2sharp in an app which uses the Win7+ User Mode Scheduler. I'm not sure if the CLR even supports this though, and until libgit2 uses 100% overlapped (i.e. async) IO, it's not even worth thinking about.

Damn it, I've nerd-sniped myself into thinking about how cool User Mode Scheduler is again. If you want to go melt your brain, check out Dave Probert talking about it.

[dahlbyk]

Whether it's a compile-time flag or a public static bool checked at runtime, 👍 for turning affinity off by default.
The neat thing about using is that if the IDisposable is null it's a noop, so DisposableThreadAffinityWrapper doesn't even need to be modified - just return null in ThreadAffinity().

[phkelley]

@dahlbyk Yes, I like that particular aspect of IDisposable, too.

@yorah Yes, I like the idea of the Proxy class even if we don't use it for these thread affinity calls anymore. It's a convenient place to put code that needs to be tightly bound to the interop. However, I've also seen it done without a separate class. Interop functions that don't need any additional help can be exposed off NativeMethods as public static extern, and those that do get marked private static extern and are instead exposed through a public static helper method that does whatever "proxy" work is necessary. The result is a lot less boilerplate code.

[dahlbyk]

@pwkelley see #203 (comment)

[dahlbyk]

How about something like this:

private static Func<IDisposable> ThreadAffinity = WithoutThreadAffinity;

internal static void DisableThreadAffinity()
{
    ThreadAffinity = WithoutThreadAffinity;
}

internal static void EnableThreadAffinity()
{
    ThreadAffinity = WithThreadAffinity;
}

private static IDisposable WithoutThreadAffinity()
{
    return null;
}

private static IDisposable WithThreadAffinity()
{
    return new DisposableThreadAffinityWrapper();
}

Not sure where to publicly expose the toggles, since Proxy is internal.

[nulltoken]

@dahlbyk 👍

Could you send a PR for this, with WithoutThreadAffinity being the default?

Not sure where to publicly expose the toggles, since Proxy is internal

Let's not expose them for now.

At some point, we'll need a way to inject a Logger, or some custom behavior (alternative backends or custom transports, for instance) in order to override LibGit2Sharp defaults. We'll pass the toggle in through this mechanism.