Skip to content

ThreadAffinity #225

Closed
Closed
ThreadAffinity#225
Milestone
v0.10.0

Description

@pwkelley
pwkelley
opened on Oct 12, 2012

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.

Activity

  1. txdv commented on Oct 12, 2012

    @txdv
    txdv
    on Oct 12, 2012
    Contributor

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

  2. nulltoken commented on Oct 12, 2012

    @nulltoken
    nulltoken
    on Oct 12, 2012
    Member

    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.

  3. joncham commented on Oct 12, 2012

    @joncham
    joncham
    on Oct 12, 2012
    Contributor

    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

  4. yorah commented on Oct 14, 2012

    @yorah
    yorah
    on Oct 14, 2012
    Contributor

    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:

    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.

  5. anaisbetts commented on Oct 14, 2012

    @anaisbetts
    anaisbetts
    on Oct 14, 2012
    Contributor

    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.

  6. anaisbetts commented on Oct 14, 2012

    @anaisbetts
    anaisbetts
    on Oct 14, 2012
    Contributor

    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.

  7. dahlbyk commented on Oct 15, 2012

    @dahlbyk
    dahlbyk
    on Oct 15, 2012
    Member

    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().

  8. phkelley commented on Oct 15, 2012

    @phkelley
    phkelley
    on Oct 15, 2012
    Member

    @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.

  9. dahlbyk commented on Oct 15, 2012

    @dahlbyk
    dahlbyk
    on Oct 15, 2012
    Member

    @pwkelley see #203 (comment)

  10. dahlbyk commented on Nov 4, 2012

    @dahlbyk
    dahlbyk
    on Nov 4, 2012
    Member

    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.

  11. nulltoken commented on Nov 17, 2012

    @nulltoken
    nulltoken
    on Nov 17, 2012
    Member

    @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.

  12. nulltoken
    closed this as completedin a6c1571on Nov 17, 2012

  13. nulltoken commented on Nov 18, 2012

    @nulltoken
    nulltoken
    on Nov 18, 2012
    Member

    Fixed by the awesome @dahlbyk with #248

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions