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

Windows: Removed thread affinity changes in sf::Clock #1007

Merged
merged 1 commit into from Nov 22, 2016

Conversation

@MarioLiebisch
Member

MarioLiebisch commented Nov 19, 2015

This should prevent timing issues on Windows XP and earlier with multi-core CPUs and broken BIOS while avoiding unnecessary threading changes for all other versions of Windows.

The whole thing has been discussed in this thread and in a few other occasions as well I think.

TL;DR: This change prevents consecutive clock/timer queries to return time stamps going back in time, while removing unnecessary changes to the current thread, which possibly weren't even applied properly (see discussion) and also could be non-effective, e.g. in case the whole process isn't assigned to the first core.

Only open question for me: What do you think? Would this need a mutex/lock for thread safety? Or would that be overkill?

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Nov 21, 2015

Member

[Code comment]
On Windows XP or older, timer inconsistencies might return an earlier time on consecutive calls to the performance counter. Let's store the previous time to ensure we don't time travel in some way, since the measured times are defined as being monotonic (non-decreasing).

When exactly does that happen? What are "timer inconsistences"? You should provide some explanation or documented source of this behavior.

The proposed solution is not really one. The returned time is still incorrect, the user is just less likely to notice it, which is not necessarily an improvement.

Furthermore, the static variable introduces race conditions into usages of sf::Clock (consider the case where a thread is interrupted before previous = now; and resumed at a later time, when now is outdated). Synchronization is not overkill, it's absolutely required to guarantee the monotonic behavior. However, both thread unsafety and synchronization overhead are quite heavy prices to pay, only to support a 14 year old OS...

Member

Bromeon commented Nov 21, 2015

[Code comment]
On Windows XP or older, timer inconsistencies might return an earlier time on consecutive calls to the performance counter. Let's store the previous time to ensure we don't time travel in some way, since the measured times are defined as being monotonic (non-decreasing).

When exactly does that happen? What are "timer inconsistences"? You should provide some explanation or documented source of this behavior.

The proposed solution is not really one. The returned time is still incorrect, the user is just less likely to notice it, which is not necessarily an improvement.

Furthermore, the static variable introduces race conditions into usages of sf::Clock (consider the case where a thread is interrupted before previous = now; and resumed at a later time, when now is outdated). Synchronization is not overkill, it's absolutely required to guarantee the monotonic behavior. However, both thread unsafety and synchronization overhead are quite heavy prices to pay, only to support a 14 year old OS...

@MarioLiebisch

This comment has been minimized.

Show comment
Hide comment
@MarioLiebisch

MarioLiebisch Nov 22, 2015

Member

Yep, easiest would be just to drop XP. ;)

Some details can be found on MSDN:

QPC is available on Windows XP and Windows 2000 and works well on most systems. However, some hardware systems' BIOS didn't indicate the hardware CPU characteristics correctly (a non-invariant TSC), and some multi-core or multi-processor systems used processors with TSCs that couldn't be synchronized across cores. Systems with flawed firmware that run these versions of Windows might not provide the same QPC reading on different cores if they used the TSC as the basis for QPC.

It's essentially like another race condition happening. There's no perfect solution. We can't manipulate subtraction for times either, as that would make other things more inaccurate as well (or simply wrong).

Theoretically we could just drop all the "adjustment" code and consider it to be potentially broken on some XP machines. There'd be no change compared to current builds, considering there's no affinity change happening in these lines. It doesn't sound right, but at the same time it doesn't introduce anything new eiher. What do you thing?

Member

MarioLiebisch commented Nov 22, 2015

Yep, easiest would be just to drop XP. ;)

Some details can be found on MSDN:

QPC is available on Windows XP and Windows 2000 and works well on most systems. However, some hardware systems' BIOS didn't indicate the hardware CPU characteristics correctly (a non-invariant TSC), and some multi-core or multi-processor systems used processors with TSCs that couldn't be synchronized across cores. Systems with flawed firmware that run these versions of Windows might not provide the same QPC reading on different cores if they used the TSC as the basis for QPC.

It's essentially like another race condition happening. There's no perfect solution. We can't manipulate subtraction for times either, as that would make other things more inaccurate as well (or simply wrong).

Theoretically we could just drop all the "adjustment" code and consider it to be potentially broken on some XP machines. There'd be no change compared to current builds, considering there's no affinity change happening in these lines. It doesn't sound right, but at the same time it doesn't introduce anything new eiher. What do you thing?

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Nov 22, 2015

Member

Some details can be found on MSDN:

You should include that link in the code, so that people who wonder can look it up. It's probably more helpful than overused time travel and back-to-the-future puns 😛

It doesn't sound right

It is incorrect, but your proposal is incorrect, too. As I said, the timer errors are just less noticeable in your case, but that doesn't make them correct. Instead, the user is misled to believe he gets correct values, although the timer is still broken. I don't see this as an advantage, especially not if it happens silently behind the scenes without documentation. I'd rather communicate potential limitations clearly to the user instead of hiding, but not fixing them.

Theoretically we could just drop all the "adjustment" code and consider it to be potentially broken on some XP machines. There'd be no change compared to current builds, considering there's no affinity change happening in these lines. It doesn't sound right, but at the same time it doesn't introduce anything new eiher. What do you thing?

Since the problem appears only on very few systems, and those systems are massively outdated, and it appears only when using sf::Clock from multiple threads, and we can still not fix the problem in those cases -- let's leave it out (possibly document the limitation) and not add a performance or even correctness penalty for 99.9% of the users.

By the way, does the previous code handle this issue correctly? If so, we can do a (runtime) check for Windows XP, and set the thread affinity mask only in that case.

Member

Bromeon commented Nov 22, 2015

Some details can be found on MSDN:

You should include that link in the code, so that people who wonder can look it up. It's probably more helpful than overused time travel and back-to-the-future puns 😛

It doesn't sound right

It is incorrect, but your proposal is incorrect, too. As I said, the timer errors are just less noticeable in your case, but that doesn't make them correct. Instead, the user is misled to believe he gets correct values, although the timer is still broken. I don't see this as an advantage, especially not if it happens silently behind the scenes without documentation. I'd rather communicate potential limitations clearly to the user instead of hiding, but not fixing them.

Theoretically we could just drop all the "adjustment" code and consider it to be potentially broken on some XP machines. There'd be no change compared to current builds, considering there's no affinity change happening in these lines. It doesn't sound right, but at the same time it doesn't introduce anything new eiher. What do you thing?

Since the problem appears only on very few systems, and those systems are massively outdated, and it appears only when using sf::Clock from multiple threads, and we can still not fix the problem in those cases -- let's leave it out (possibly document the limitation) and not add a performance or even correctness penalty for 99.9% of the users.

By the way, does the previous code handle this issue correctly? If so, we can do a (runtime) check for Windows XP, and set the thread affinity mask only in that case.

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Jan 1, 2016

Member

[...] let's leave [the checks and "corrections"] out (possibly document the limitation) and not add a performance or even correctness penalty for 99.9% of the users.

By the way, does the previous code handle this issue correctly? If so, we can do a (runtime) check for Windows XP, and set the thread affinity mask only in that case.

Comments on that? Should we proceed with the implementation?

Member

Bromeon commented Jan 1, 2016

[...] let's leave [the checks and "corrections"] out (possibly document the limitation) and not add a performance or even correctness penalty for 99.9% of the users.

By the way, does the previous code handle this issue correctly? If so, we can do a (runtime) check for Windows XP, and set the thread affinity mask only in that case.

Comments on that? Should we proceed with the implementation?

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Jan 3, 2016

Member

Sounds like a plan. What do you think, @MarioLiebisch?

Member

eXpl0it3r commented Jan 3, 2016

Sounds like a plan. What do you think, @MarioLiebisch?

@MarioLiebisch

This comment has been minimized.

Show comment
Hide comment
@MarioLiebisch

MarioLiebisch Jan 18, 2016

Member

Completely missed the latest comments. While I could agree with what Bromeon commented above, I'd still vote to completely remove the whole "bugfix", considering:

  • As it's been stated, only very few boards on a no longer supported OS (by Microsoft) are affected.
  • Only a specific use case is actually affected (running on multicore machine).
  • Even with the thread affinity mask being set there's a slim chance timing will be screwed (see MSDN notes).
Member

MarioLiebisch commented Jan 18, 2016

Completely missed the latest comments. While I could agree with what Bromeon commented above, I'd still vote to completely remove the whole "bugfix", considering:

  • As it's been stated, only very few boards on a no longer supported OS (by Microsoft) are affected.
  • Only a specific use case is actually affected (running on multicore machine).
  • Even with the thread affinity mask being set there's a slim chance timing will be screwed (see MSDN notes).
@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Jan 18, 2016

Member

While I could agree with what Bromeon commented above, I'd still vote to completely remove the whole "bugfix"

Why "still"? I was in favor of removing this "bugfix" (which doesn't fix anything, but hide bugs, decrease performance and introduce race conditions -- see above).

If it's easy to support the special case for Windows XP by using the current implementation (not this PR), I'm okay with a case differentiation. Otherwise let's keep things simple and forget about it.

Member

Bromeon commented Jan 18, 2016

While I could agree with what Bromeon commented above, I'd still vote to completely remove the whole "bugfix"

Why "still"? I was in favor of removing this "bugfix" (which doesn't fix anything, but hide bugs, decrease performance and introduce race conditions -- see above).

If it's easy to support the special case for Windows XP by using the current implementation (not this PR), I'm okay with a case differentiation. Otherwise let's keep things simple and forget about it.

@MarioLiebisch

This comment has been minimized.

Show comment
Hide comment
@MarioLiebisch

MarioLiebisch Jan 18, 2016

Member

Thread affinity just hides part of the problem, but doesn't completely fix it (there's still some tiny error margin left) as far as I'm aware. Not to forget that the current (master branch) implementation is obviously broken as well (since there's no context switch happening, even if needed).

Member

MarioLiebisch commented Jan 18, 2016

Thread affinity just hides part of the problem, but doesn't completely fix it (there's still some tiny error margin left) as far as I'm aware. Not to forget that the current (master branch) implementation is obviously broken as well (since there's no context switch happening, even if needed).

@JonnyPtn

This comment has been minimized.

Show comment
Hide comment
@JonnyPtn

JonnyPtn Jan 18, 2016

Contributor

Quick one from me - I've tried setting the process affinity here, and I can still reproduce the problem.

Contributor

JonnyPtn commented Jan 18, 2016

Quick one from me - I've tried setting the process affinity here, and I can still reproduce the problem.

@JonnyPtn

This comment has been minimized.

Show comment
Hide comment
@JonnyPtn

JonnyPtn Jan 27, 2016

Contributor

Following up on this - We've worked around the issue by placing the call to QueryPerformanceCounter inside a critical section (https://msdn.microsoft.com/en-us/library/windows/desktop/ms682530(v=vs.85).aspx). This appears to prevent the issue happening on our XP test machines.

Not really suggesting this gets implemented, as it's only necessary on XP machines (and not even all of them as far as I can tell), but hopefully helps you to find a more suitable fix and decide whether it's worth implementing or not.

Contributor

JonnyPtn commented Jan 27, 2016

Following up on this - We've worked around the issue by placing the call to QueryPerformanceCounter inside a critical section (https://msdn.microsoft.com/en-us/library/windows/desktop/ms682530(v=vs.85).aspx). This appears to prevent the issue happening on our XP test machines.

Not really suggesting this gets implemented, as it's only necessary on XP machines (and not even all of them as far as I can tell), but hopefully helps you to find a more suitable fix and decide whether it's worth implementing or not.

@TankOs

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs Jan 28, 2016

Member

@JonnyPtn Great that you are able to workaround the issue.

Given that XP is really old, and that adding a critical section wouldn't benefit any other platform, I think the most SFML should do is documenting it.

What do others think?

Member

TankOs commented Jan 28, 2016

@JonnyPtn Great that you are able to workaround the issue.

Given that XP is really old, and that adding a critical section wouldn't benefit any other platform, I think the most SFML should do is documenting it.

What do others think?

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Feb 18, 2016

Member

So what do we decide here?

Member

eXpl0it3r commented Feb 18, 2016

So what do we decide here?

@MarioLiebisch

This comment has been minimized.

Show comment
Hide comment
@MarioLiebisch

MarioLiebisch Feb 18, 2016

Member

I'd consider documenting it (short note and linking the MSDN article(s)) and then removing the adjustments. Support wise, Windows XP is essentially dead. When using MSVC you even have to jump hoops (installing support, changing project settings etc.) to still even compile for it.

Member

MarioLiebisch commented Feb 18, 2016

I'd consider documenting it (short note and linking the MSDN article(s)) and then removing the adjustments. Support wise, Windows XP is essentially dead. When using MSVC you even have to jump hoops (installing support, changing project settings etc.) to still even compile for it.

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Feb 18, 2016

Member

It seems like we agree that:

  • the thread affinity in the current codebase should be removed
  • the code proposed by this PR doesn't fix the problem but breaks even more things

So the question is whether we go with the simple most code (only QueryPerformanceCounter) or implement the Windows XP fix as suggested by @JonnyPtn. Other than @TankOs and @MarioLiebisch, since we still have Windows XP users (even among the few people who use SFML commercially!), I consider a case differentiation appropriate. Checking for the Windows version is quite simple, and we could even use a sf::Mutex instead of critical sections (but we're already in WinAPI code anyway).

Member

Bromeon commented Feb 18, 2016

It seems like we agree that:

  • the thread affinity in the current codebase should be removed
  • the code proposed by this PR doesn't fix the problem but breaks even more things

So the question is whether we go with the simple most code (only QueryPerformanceCounter) or implement the Windows XP fix as suggested by @JonnyPtn. Other than @TankOs and @MarioLiebisch, since we still have Windows XP users (even among the few people who use SFML commercially!), I consider a case differentiation appropriate. Checking for the Windows version is quite simple, and we could even use a sf::Mutex instead of critical sections (but we're already in WinAPI code anyway).

@MarioLiebisch

This comment has been minimized.

Show comment
Hide comment
@MarioLiebisch

MarioLiebisch Feb 18, 2016

Member

Looks neat. I'm still not sure why Jonny's code fixes the issue though. Before I thought because it prevents context switches, but that's obviously no longer the case for most modern versions of Windows.

Member

MarioLiebisch commented Feb 18, 2016

Looks neat. I'm still not sure why Jonny's code fixes the issue though. Before I thought because it prevents context switches, but that's obviously no longer the case for most modern versions of Windows.

@JonnyPtn

This comment has been minimized.

Show comment
Hide comment
@JonnyPtn

JonnyPtn Feb 19, 2016

Contributor

I'm hoping windows support is something that we won't have to worry about in the not too distant future. From a non-commercial user point of view, my opinion would be that SFML should consider dropping XP support (but documenting the fix as @MarioLiebisch said), as I imagine it will become more hassle than it's worth. From a commercial point of view we use a lightly modified version of SFML anyway which is rarely (if ever) up to date with master, so no pressure on that front to carry on supporting it.

Contributor

JonnyPtn commented Feb 19, 2016

I'm hoping windows support is something that we won't have to worry about in the not too distant future. From a non-commercial user point of view, my opinion would be that SFML should consider dropping XP support (but documenting the fix as @MarioLiebisch said), as I imagine it will become more hassle than it's worth. From a commercial point of view we use a lightly modified version of SFML anyway which is rarely (if ever) up to date with master, so no pressure on that front to carry on supporting it.

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Feb 19, 2016

Member

Here is what I came up with based on what has been discussed:

////////////////////////////////////////////////////////////
// Headers
////////////////////////////////////////////////////////////
#include <SFML/System/Win32/ClockImpl.hpp>
#include <SFML/System/Mutex.hpp>
#include <SFML/System/Lock.hpp>
#include <windows.h>


namespace
{
    sf::Mutex oldWindowsMutex;

    LARGE_INTEGER getFrequency()
    {
        LARGE_INTEGER frequency;
        QueryPerformanceFrequency(&frequency);
        return frequency;
    }

    bool isWindowsXpOrOlder()
    {
        HMODULE kernel32 = GetModuleHandleA("Kernel32.dll");

        // If loading Kernel32.dll fails (should never happen) assume Vista or newer (almost always the case)
        if (!kernel32)
            return false;

        // GetProductInfo is only supported from Vista onwards, so we use its absence as an indicator for XP
        // (see http://msdn.microsoft.com/en-us/library/ms724358(v=vs.85).aspx)
        return (GetProcAddress(kernel32, "GetProductInfo") == NULL);
    }
}

namespace sf
{
namespace priv
{
////////////////////////////////////////////////////////////
Time ClockImpl::getCurrentTime()
{
    // Get the frequency of the performance counter
    // (it is constant across the program lifetime)
    static LARGE_INTEGER frequency = getFrequency();

    // Detect if we are on Windows XP or older
    static bool oldWindows = isWindowsXpOrOlder();

    LARGE_INTEGER time;

    if (oldWindows)
    {
        // Acquire a lock (CRITICAL_SECTION) to prevent travelling back in time
        Lock lock(oldWindowsMutex);

        // Get the current time
        QueryPerformanceCounter(&time);
    }
    else
    {
        // Get the current time
        QueryPerformanceCounter(&time);
    }

    // Return the current time as microseconds
    return sf::microseconds(1000000 * time.QuadPart / frequency.QuadPart);
}

} // namespace priv

} // namespace sf

Feel free to modify it and use it to update this PR.

Member

binary1248 commented Feb 19, 2016

Here is what I came up with based on what has been discussed:

////////////////////////////////////////////////////////////
// Headers
////////////////////////////////////////////////////////////
#include <SFML/System/Win32/ClockImpl.hpp>
#include <SFML/System/Mutex.hpp>
#include <SFML/System/Lock.hpp>
#include <windows.h>


namespace
{
    sf::Mutex oldWindowsMutex;

    LARGE_INTEGER getFrequency()
    {
        LARGE_INTEGER frequency;
        QueryPerformanceFrequency(&frequency);
        return frequency;
    }

    bool isWindowsXpOrOlder()
    {
        HMODULE kernel32 = GetModuleHandleA("Kernel32.dll");

        // If loading Kernel32.dll fails (should never happen) assume Vista or newer (almost always the case)
        if (!kernel32)
            return false;

        // GetProductInfo is only supported from Vista onwards, so we use its absence as an indicator for XP
        // (see http://msdn.microsoft.com/en-us/library/ms724358(v=vs.85).aspx)
        return (GetProcAddress(kernel32, "GetProductInfo") == NULL);
    }
}

namespace sf
{
namespace priv
{
////////////////////////////////////////////////////////////
Time ClockImpl::getCurrentTime()
{
    // Get the frequency of the performance counter
    // (it is constant across the program lifetime)
    static LARGE_INTEGER frequency = getFrequency();

    // Detect if we are on Windows XP or older
    static bool oldWindows = isWindowsXpOrOlder();

    LARGE_INTEGER time;

    if (oldWindows)
    {
        // Acquire a lock (CRITICAL_SECTION) to prevent travelling back in time
        Lock lock(oldWindowsMutex);

        // Get the current time
        QueryPerformanceCounter(&time);
    }
    else
    {
        // Get the current time
        QueryPerformanceCounter(&time);
    }

    // Return the current time as microseconds
    return sf::microseconds(1000000 * time.QuadPart / frequency.QuadPart);
}

} // namespace priv

} // namespace sf

Feel free to modify it and use it to update this PR.

@mantognini mantognini added this to the 2.4.1 milestone Aug 9, 2016

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Aug 10, 2016

Member

Can you adjust the PR, @MarioLiebisch?
And maybe @JonnyPtn can you give the new code a try?

Member

eXpl0it3r commented Aug 10, 2016

Can you adjust the PR, @MarioLiebisch?
And maybe @JonnyPtn can you give the new code a try?

@MarioLiebisch

This comment has been minimized.

Show comment
Hide comment
@MarioLiebisch

MarioLiebisch Aug 10, 2016

Member

Updated and rebased on current master.

Member

MarioLiebisch commented Aug 10, 2016

Updated and rebased on current master.

@eXpl0it3r eXpl0it3r modified the milestones: 2.5, 2.4.1 Aug 13, 2016

@Fytch

This comment has been minimized.

Show comment
Hide comment
@Fytch

Fytch Nov 13, 2016

Contributor

While profiling my program in order to track down the issue that caused massive input lags, I stumbled across this PR. As can be seen in this screenshot, SetThreadAffinityMask was responsible for over 30% of the main thread's run time. Simply removing the concerned line that invoked this function in Win32/ClockImpl.cpp without any substitute seemed to fix this problem for me.

Conditions: Windows 7 SP1 64-bit, SFML master branch, GCC 6.2.0.

I think this problem's urgency is being vastly understated here. The problem has been localized for a year now and it still hasn't been fixed, neither in the official release nor in the master branch. This single line causes massive lags and heavy CPU utilization (30% in my particular case) on Windows systems only to provide better support for an obsolete operating system in combination with a broken BIOS. This is a very bad trade-off in my opinion. As many SFML users on Windows are affected by this, knowingly or unknowingly, this issue should be fixed ASAP and be part of 2.4.2.

Contributor

Fytch commented Nov 13, 2016

While profiling my program in order to track down the issue that caused massive input lags, I stumbled across this PR. As can be seen in this screenshot, SetThreadAffinityMask was responsible for over 30% of the main thread's run time. Simply removing the concerned line that invoked this function in Win32/ClockImpl.cpp without any substitute seemed to fix this problem for me.

Conditions: Windows 7 SP1 64-bit, SFML master branch, GCC 6.2.0.

I think this problem's urgency is being vastly understated here. The problem has been localized for a year now and it still hasn't been fixed, neither in the official release nor in the master branch. This single line causes massive lags and heavy CPU utilization (30% in my particular case) on Windows systems only to provide better support for an obsolete operating system in combination with a broken BIOS. This is a very bad trade-off in my opinion. As many SFML users on Windows are affected by this, knowingly or unknowingly, this issue should be fixed ASAP and be part of 2.4.2.

@MarioLiebisch

This comment has been minimized.

Show comment
Hide comment
@MarioLiebisch

MarioLiebisch Nov 14, 2016

Member

Hm... I actually thought this would have been merged already. Somehow dropped off the radar.

Member

MarioLiebisch commented Nov 14, 2016

Hm... I actually thought this would have been merged already. Somehow dropped off the radar.

@MarioLiebisch MarioLiebisch self-assigned this Nov 14, 2016

@MarioLiebisch

This comment has been minimized.

Show comment
Hide comment
@MarioLiebisch

MarioLiebisch Nov 14, 2016

Member

@Fytch did you try this branch and does it solve the problem? As far as I remember the main problem with this branch was the lack of feedback/testers.

Member

MarioLiebisch commented Nov 14, 2016

@Fytch did you try this branch and does it solve the problem? As far as I remember the main problem with this branch was the lack of feedback/testers.

@Fytch

This comment has been minimized.

Show comment
Hide comment
@Fytch

Fytch Nov 15, 2016

Contributor

@MarioLiebisch I cloned, compiled and tested the branch bugfix/windows-clock-threading just now. Everything works and the costly SetThreadAffinityMask calls have disappeared.

Test system: Windows 7 SP1 64-bit, MinGW GCC 6.2.0.

Profiler: VerySleepy CS 0.90 64-bit

Contributor

Fytch commented Nov 15, 2016

@MarioLiebisch I cloned, compiled and tested the branch bugfix/windows-clock-threading just now. Everything works and the costly SetThreadAffinityMask calls have disappeared.

Test system: Windows 7 SP1 64-bit, MinGW GCC 6.2.0.

Profiler: VerySleepy CS 0.90 64-bit

@eXpl0it3r eXpl0it3r modified the milestones: 2.4.2, 2.5 Nov 16, 2016

@eXpl0it3r eXpl0it3r changed the base branch from master to 2.4.x Nov 16, 2016

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Nov 16, 2016

Member

@MarioLiebisch Can you rebase this onto 2.4.x instead of master?

Member

eXpl0it3r commented Nov 16, 2016

@MarioLiebisch Can you rebase this onto 2.4.x instead of master?

Windows: Removed thread affinity changes in sf::Clock
* This should prevent timing issues on Windows XP and earlier with broken BIOS while avoiding unnecessary threading changes.
@MarioLiebisch

This comment has been minimized.

Show comment
Hide comment
@MarioLiebisch
Member

MarioLiebisch commented Nov 16, 2016

Done

@eXpl0it3r

I've talked to Jonny and he can't test this one their (faulty) Windows XP box. It otherwise seems to run fine on Windows XP and the GetVersion() check also seems to work. As such this can be merged.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Nov 21, 2016

Member

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.

Member

eXpl0it3r commented Nov 21, 2016

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.

@eXpl0it3r eXpl0it3r merged commit da3632b into 2.4.x Nov 22, 2016

15 checks passed

android-armeabi-v7a-api13 Build #54 done.
Details
debian-gcc-64 Build #328 done.
Details
freebsd-gcc-64 Build #290 done.
Details
osx-clang-el-capitan Build #174 done.
Details
static-analysis Build #297 done.
Details
windows-gcc-492-tdm-32 Build #186 done.
Details
windows-gcc-492-tdm-64 Build #183 done.
Details
windows-gcc-610-mingw-32 Build #119 done.
Details
windows-gcc-610-mingw-64 Build #119 done.
Details
windows-vc11-32 Build #298 done.
Details
windows-vc11-64 Build #295 done.
Details
windows-vc12-32 Build #296 done.
Details
windows-vc12-64 Build #295 done.
Details
windows-vc14-32 Build #296 done.
Details
windows-vc14-64 Build #299 done.
Details

@eXpl0it3r eXpl0it3r deleted the bugfix/windows-clock-threading branch Nov 22, 2016

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