[llvm-commits] [PATCH] Improved threading support on Windows

[Aaron Ballman]

Ping?

On Tue, Aug 23, 2011 at 5:19 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Tue, Aug 23, 2011 at 5:34 AM, NAKAMURA Takumi <geek4civic at gmail.com> wrote:
>> Good evening, Aaron!
>>
>> About lib/Support/Windows/RWMutex.inc;
>>
>>  - Would you like to try describing rwmutex on windows xp?
>>    Oh yeah, I don't have any Windows XP hosts any more. :(
>
> In code or on the mailing list?
>
> Windows XP doesn't have slim reader/writer locks (they're a Vista and
> up API).  So the RWMutex object supports reader/writer locks when
> possible, but fallsback on the more heavy-handed original
> implementation, which uses vanilla critical section objects.  This
> implementation is correct on XP, just not as efficient as on Vista and
> higher.  On Vista and up, you can have multiple non-blocked readers at
> once, but on XP you will only be able to have one reader at a time.
>
> If you'd like me to update the comments in the code, I certainly can.
>
>>  - I think, rather to refer to kernel32.dll, GetModuleHandle(NULL)
>> would be more enough.
>>    How do you think?
>
> I've never been too happy with that convention.  While it would
> certainly work because Kernel32 is mapped into every executable's
> process space, I don't think the code would be as clear where the
> functions are coming from.  Also, there's no performance penalty for
> calling LoadLibrary vs GetModuleHandle( NULL ) that I've ever heard
> of.  Since the library is already loaded into memory, the loader will
> traverse the list of loaded modules in the PEB, see Kernel32 (early
> on) and return the module handle from there.
>
>>  - It would be happier for us to have generic "delayed dll resolver"
>> for NT5.1-unavailable entries.
>>    I think it might be the global ctor. How do you think?
>
> That's not a bad idea.  I did a quick search of the code base, and
> this is the first case we're using lazy loading for OS APIs.  My
> personal feeling is: if we need to do this a second time, a helper API
> could be designed to make this cleaner.  But since this is only
> happening once, the helper may be overkill.
>
>>  - How about to split RWMutexImpl to Windows XP and higher?
>
> I originally looked into that, but it would be a larger change than I
> was comfortable with.  Right now, the Impl is a concrete class, and an
> OS version split would require it to be a bridge.  If you think it's a
> better approach though, I can certainly tackle it.
>
>>  - Could you consider unittests for rwmutex?
>
> Yes, I could probably write some up, at least for testing them on
> Windows.  Can you recommend where I should put the unit tests though
> (I've not yet done something like this for LLVM).
>
> Thanks for the review!
>
> ~Aaron
>