1417234 - Use SRWLock as Mutex for mozjemalloc on Windows

Status: RESOLVED FIXED

(Core :: Memory Allocator, enhancement)

Comment 1

[Mike Hommey [:glandium]]

Attachment: Bug 1417234 - Use SRWLock as Mutex for mozjemalloc on Windows.

SRWLock is more lightweight than CriticalSection, but is only available
on Windows Vista and more. So until we actually dropped support Windows
XP, we had to use CriticalSection.

Now that all supported Windows versions do have SRWLock, this is a
switch we can make, and not only because SRWLock is more lightweight,
but because it can be statically initialized like on other platforms,
allowing to use the same initialization code as on other platforms,
and removing the requirement for a DllMain, which in turn can allow
to statically link mozjemalloc in some cases, instead of requiring a
shared library (DllMain only works on shared libraries), or manually
call the initialization function soon enough.

Review commit: https://reviewboard.mozilla.org/r/199518/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/199518/

Comment 2

[Mike Hommey [:glandium]]

Comment on attachment 8928302 [details]
Bug 1417234 - Use SRWLock as Mutex for mozjemalloc on Windows.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/199518/diff/1-2/

Comment 3

[Masatoshi Kimura [:emk]]

Comment on attachment 8928302 [details]
Bug 1417234 - Use SRWLock as Mutex for mozjemalloc on Windows.

https://reviewboard.mozilla.org/r/199518/#review204666

::: memory/build/mozjemalloc.cpp
(Diff revision 2)
> -{
> -  switch (reason) {
> -    case DLL_PROCESS_ATTACH:
> -      // Don't force the system to page DllMain back in every time
> -      // we create/destroy a thread
> -      DisableThreadLibraryCalls(hModule);

DisableThreadLibraryCalls should be called regardless of this change.

Comment 4

[Nicholas Nethercote [inactive]]

Comment on attachment 8928302 [details]
Bug 1417234 - Use SRWLock as Mutex for mozjemalloc on Windows.

https://reviewboard.mozilla.org/r/199518/#review204672

Ok by me. I don't know enough about DisableThreadLibraryCalls() to have an opinion on emk's suggestion.

Comment 5

[Mike Hommey [:glandium]]

(In reply to Masatoshi Kimura [:emk] from comment #3)
> Comment on attachment 8928302 [details]
> Bug 1417234 - Use SRWLock as Mutex for mozjemalloc on Windows.
> 
> https://reviewboard.mozilla.org/r/199518/#review204666
> 
> ::: memory/build/mozjemalloc.cpp
> (Diff revision 2)
> > -{
> > -  switch (reason) {
> > -    case DLL_PROCESS_ATTACH:
> > -      // Don't force the system to page DllMain back in every time
> > -      // we create/destroy a thread
> > -      DisableThreadLibraryCalls(hModule);
> 
> DisableThreadLibraryCalls should be called regardless of this change.

DllMain is only ever called for the dll it resides in. Meaning that DisableThreadLibraryCalls is only ever called here for the dll mozjemalloc resides in.

DisableThreadLibraryCalls avoids that DllMain function to be called for DLL_THREAD_ATTACH and DLL_THREAD_DETACH notifications. The only other thing that this DllMain function did is call malloc_init_hard, which goes away. Thus there's nothing left for that DllMain to do in any situation. Removing the function makes it never called, which is a superset of what DisableThreadLibraryCalls did.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5b8cbab9374f
Use SRWLock as Mutex for mozjemalloc on Windows. r=njn

Comment 7

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/5b8cbab9374f

Comment 8

[Sylvestre Ledru [:Sylvestre]]

https://hg.mozilla.org/projects/oak/rev/5b8cbab9374f065bf4cac694d5ad251f2134bb37
Bug 1417234 - Use SRWLock as Mutex for mozjemalloc on Windows. r=njn