oneapi-src / oneTBB

oneAPI Threading Building Blocks (oneTBB)
https://oneapi-src.github.io/oneTBB/
Apache License 2.0
5.68k stars 1.02k forks source link

Compiling in Windows with Unicode Enabled #921

Closed JohnJScott closed 3 months ago

JohnJScott commented 2 years ago

Downloaded, compiled, implemented -> thread contention fixed and our app runs about 20x quicker!

The only code change I made was fixing some API calls in dynamic_link.cpp when Unicode is enabled.

GetModuleHandleEx -> GetModuleHandleExA LoadLibrary -> LoadLibraryA GetModuleFileName -> GetModuleFileNameA

The reason I enabled Unicode is to follow the conventions mentioned here - https://utf8everywhere.org/

My workaround works in my case, but is no means a fix. I have minimal resources to test any changes on platforms other than Windows, so I'd thought I raise a flag and let the big guys make a recommendation.

Cheers John

isaevil commented 2 years ago

@JohnJScott I don't quite understand what you mean. Could you please share more details?

JohnJScott commented 2 years ago

By default, LoadLibrary (for example) is macro'd to be LoadLibraryW( wide string ) when the unicode character set is enabled, and LoadLibraryA( multibyte string ) when the multibyte character set is enabled.

Following the conventions mentioned in https://utf8everywhere.org/ the best practice is to use utf8 everywhere, enable the unicode character set (image below), and convert utf8 parameters to wide characters on demand.

If I enable unicode in TBB, the above fixes need to be made for it to compile.

I did this after cmake has done its magic. The overridden terms here do not help; Microsoft multibyte character set acts like utf8 (I think it's a subset), and Unicode could either be utf16 or ucs2.

Potential action items for team TBB:

Overall, it's a two fold operation 1. Make it work, 2. Make the ideological decision whether to go with single byte characters or wide char characters.

One of my other projects (UTF16MustDIE) makes my opinion clear =)

Cheers John

image

nofuturre commented 3 months ago

@JohnJScott is this issue still relevant?

IderaJohn commented 3 months ago

Not to me - we went with mimalloc in the end to avoid a dll dependency. Also, I've moved jobs.

I haven't looked at any TBB updates, so I assume this hasn't changed?

It is an easy fix to make it compile with or without UNICODE defined (just call the A versions of the Windows libraries directly).

However, this is the proper solution per utf8everywhere.org:

nofuturre commented 3 months ago

If anyone encounter this issue in the future please open new issue with a link to this one