Closed Bug 52725 Opened 24 years ago Closed 21 years ago

Prevent winMM.dll from statically linking to mozilla


(SeaMonkey :: General, defect, P2)

Windows 2000


(Not tracked)



(Reporter: rickg, Assigned: cathleennscp)



(Keywords: perf)


(1 file)

The winMM.dll is unnecessarily linked to mozilla. There are only 2 uses: nspr 
and nsWindow. I have a patch to fix nsWindow to use WINMM.DLL dynamically, and 
the nspr group has agreed to eliminate their dependency. By fixing this (and a 
related bug involving IMM32) we can eliminate 700K from loading with mozilla. 
It's a start.
Requesting approval to land for beta3. This prevents a system DLL from loading 
until needed, and saves memory. 
Keywords: nsbeta3
Whiteboard: [nsbeta3]
Target Milestone: --- → M18
what is the patch?
I shipped the main code in the patch to the porkjockey's newgroup. Here's the 
actual patch. Note that you also have to remove WINMM.DLL from the windows 

> ////////////////////////////////////////////////////////////////////////
> class CWinMM {
>   typedef int (CALLBACK *PlayPtr)(const char*,HMODULE,DWORD);
> public:
>   CWinMM(const char* aModuleName="WINMM.DLL") {
>     mInstance=::LoadLibrary(aModuleName);
>     mPlay=(mInstance) ? (PlayPtr)GetProcAddress(mInstance,"PlaySound") : 0;
>   }
>   ~CWinMM() {
>     if(mInstance)
>       ::FreeLibrary(mInstance);
>     mInstance=0;
>     mPlay=0;
>   }
>   BOOL PlaySound(const char *aSoundFile,HMODULE aModule,DWORD aOptions) {
>     return (mPlay) ? mPlay(aSoundFile, aModule, aOptions) : FALSE;
>   }
> private:
>   HINSTANCE mInstance;
>   PlayPtr mPlay;
> };
> static CWinMM& GetModuleWinMM() {
>   static CWinMM gSharedWinMM;  //construct this only after you *really* need i
>   return gSharedWinMM;
> }
> ////////////////////////////////////////////////////////////////////////
> #if 1
> #else
>       //Rather than link WINMM statically, let's grab it only when we need it.

>       //This will help startup time, and will only affect startup performance
>       //marginally, the first time the method is invoked.
>     CWinMM &theMM=GetModuleWinMM();
>     theMM.PlaySound(string, nsnull, (SND_MEMORY | SND_NODEFAULT | SND_SYNC));
> #endif
requesting beta3+ status.
Priority: P3 → P2
Whiteboard: [nsbeta3] → [nsbeta3+]
PDT agrees P2
Whiteboard: [nsbeta3+] → [nsbeta3+][PDTP2]
The NSPR patch can be found in Bug #42714.  It is included
with NSPR 4.1 (released).
Attached patch Patch for NSPR.Splinter Review on that NSPR patch.

partial fix landed. The nsSound file dependency has be patched, and we're
waiting the patch from nspr (see attached). Syd will confirm on AIM.
The NSPR winmm.dll fix has been checked in on
NSPRPUB_CLIENT_BRANCH.  (See bug #42714.)
PDT marking nsbeta3-, nominating for rtm
Keywords: rtm
Whiteboard: [nsbeta3+][PDTP2] → [nsbeta3-][PDTP2]
I just wanted to note that the NSPR winmm.dll fix is
in the Netscape_20000922_BRANCH.
Hang on, PDT. The changes are in and being verified so that I can remove the 
library. Marking RTM.
Keywords: nsbeta3
Whiteboard: [nsbeta3-][PDTP2] → [rtm+][PDTP2]
Marking ++ for review!
Whiteboard: [rtm+][PDTP2] → [rtm++][PDTP2]
Whiteboard: [rtm++][PDTP2] → [rtm+][PDTP2]
Rick, please let us know what has been landed where.  Also, you need to have
your patched reviewed and super reviewed.
Whiteboard: [rtm+][PDTP2] → [rtm+ needinfo][PDTP2]
Whiteboard: [rtm+ needinfo][PDTP2] → [rtm+ need info][PDTP2]
This code was reviewed by several folks (attinasi, syd) and super-reviewed by 

The win (as Michael suggests) is that we would defer the loading of this DLL 
until AIM actually tried to play a sound (I don't think any other code uses this 
library). It saves 500K on startup, best case.

The only remaining change is to strip the winmm.lib statement from the

! winmm.lib

Whiteboard: [rtm+ need info][PDTP2] → [rtm+][PDTP2]
PDT marking [rtm++]
Whiteboard: [rtm+][PDTP2] → [rtm++][PDTP2]
What is the status of this bug?  It was approved for branch landing 8 days
ago... but there were no further comments?
Reading this, I can't figure out what was landed, and what was not landed.
Similarly, I can't figure out what needs to be landed.  Pushing back to rtm need
info until we have clarity.
Please re-nominate to rtm-plus when there is a reviewed plan for landing
something (additional?)
Whiteboard: [rtm++][PDTP2] → [rtm need info][PDTP2]
Perhaps I wasn't clear enough when I said that all we need to do now was remove
winmm.lib from the makefile. But there you have it.

I referred to this bug when I spoke to the PDT (Phil/Jar) last Thursday. They
were clear that bugs of this type would not be accepted. If they've changed
their mind, I'm happy to land the tiny change.

Whiteboard: [rtm need info][PDTP2] → [rtm+][PDTP2]
PDT says rtm-, benefit/risk ratio not high enough for this late in the release
Whiteboard: [rtm+][PDTP2] → [rtm-][PDTP2]
Target Milestone: M18 → ---
Removing [PDT] grafitti.
Whiteboard: [rtm-][PDTP2]
Rick's gone. Bouncing to cathleen.
Assignee: rickg → cathleen
Keywords: perf
What is the status here ?
see comment #19.
Sounds like we're trying to delay load winmm.dll till it's needed by aim, for
startup performance improvement.

Syd, do you know anything about this?
I'm confused regarding to what's been done, and what's left to do.

This is what's in the mozilla tree currently:

Blocks: 71781
Target Milestone: --- → mozilla1.2
i think this is fixed the last outstanding reference listed was a 
which doesn't appear in my lxr search.

what does appear is some cldap, nss and configure references but i think they 
could be handled in a new bug.
LXR shows winmm.dll only in /widget/src/windows/nsSound.cpp, not in any
makefiles. So I think this bug can be closed.
Closed: 21 years ago
Resolution: --- → FIXED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.