Move enumerable_thread_specific Windows.h usage to cpp

[Chronial]

Description

This change gets rid of the Windows.h include in the public enumerable_thread_specific .h by moving it a cpp.

Fixes #573

Type of change

• bug fix - change which fixes an issue
• new feature - change which adds functionality
• tests - change in tests
• infrastructure - change in infrastructure and CI
• documentation - documentation update

Tests

• added - required for new features and for some bug fixes
• not needed

Documentation

• updated in # - add PR number
• needs to be updated
• not needed

Breaks backward compatibility

• Yes
• No
• Unknown

[alexey-katranov]

The patch raises two questions:

1. Can it affect performance?
2. Are we ready to pay maintenance cost for additional entry points?

Before answering above questions, can we consider the option to use processthreadsapi.h (or fibersapi.h with __TBB_WIN8UI_SUPPORT)?

[Chronial]

Can it affect performance?

The Tls* and Fls* functions are themselves dllimport, so I wouldn't expect this change to have a significant impact.

Before answering above questions, can we consider the option to use processthreadsapi.h (or fibersapi.h with __TBB_WIN8UI_SUPPORT)?

I'm not sure whether you're supposed to include processthreadsapi.h without windows.h. If we want to do that, we would have to replicate the architecture detection block from windows.h because processthreadsapi.h needs one of these symbols to be defined.

If you would prefer that, I can also create a PR for using processthreadsapi.h. But I do feel a bit uneasy about copying that architecture code.

[anton-potapov]

@Chronial is there any estimates or real measurements of build times reduction effect of this patch ?

[Chronial]

This change won't have a massive impact on our build times. In places where this is a serious issue, enumerable_thread_specific.h is simply added to the precompiled headers.

This is more about usability:

• Windows.h includes are affected by various macros, so it should be included before some and after some other headers. This now affects enumerable_thread_specific.h, which makes it more error-prone to use.
• Windows.h defines countless names as macros, like SetEvent, SetWindowText, RegisterClass and many more. This makes these identifiers unusable in any file that includes Windows.h. This doesn't just affect global names, but also as method names and names in any namespace. This property is now inherited by enumerable_thread_specific.
• Windows.h notoriously defines the names min and max as macros, breaking std::min and std::max with nice error messages.
• Due to the points above and the size of Windows.h, you are strongly discouraged from including enumerable_thread_specific in any of our headers. So enumerable_thread_specific has to be hidden with pimpl or simply can't be used in templates.

[Chronial]

@alexey-katranov Is there anything I can do to move this forward?

[Chronial]

ping :)

[anton-potapov]

@pavelkumbrasev , I believe now it's your chance to review it :)