The service manager has APIs that are protected by a monitor, as well as using a threadsafe hashtable for internal use. Travis said "service manager is dying", so I'm handing him this bundle o' joy to remind him that the new incarnation shouldn't do that.
This probably belongs to you dp as you are the one working on that side of the service manager.
It might take a while before the servicemanager goes away. In the mean time, any suggestions for improvement here.
This is a performance bottleneck. We should see if we can eliminate the excessive locking.
If the service manager entry points are protected by a lock, the hashtable doesn't need to use one. I see line 180 of nsServiceManager.cpp says "Get a threadSafe hashtable" which may not be necessary any more. I'd investigate setting that parameter to false.
Just curious, why are you eliminating the other case: remove locks from apis and protect the data (hashtable entries).
performance team should nominate this to make it happen.
Correction, performance team should nominate this IF they want to make it happen.
Performance guys, can you evaluate this and let us know if we should fix it.
whatever. We are going to leave it like this for a while.
Moving "perf" to keyword field. This is the are we use now :-)
moving from leftover dogfood to beta1 radar per beta criteria priority #2
Need to quanify what impact this has on the spec and where you think it will help and how much please.
Where is the spec?
This bug came out of the timing analysis done by the performance effort. I am sure this was a blaring number and hence the bug. We need to quantify to really get the number by which this is a problem and further by how much we can reduce it since there is no way we can eliminate all the locks. I know this doesn't help much. Just thinking aloud at this point. My general sense was that this wont improve performance a great deal. So let us go after bigger fish...
Putting on PDT- radar for beta1. If you have date to make a better call, let us know - pdt
A recent Quantify run shows that GetService accounts for 15.18% of total runtime. The enter/exit monitors there account for about 2% of that. The service manager also uses a threadsafe hashtable, which accounts for another 1%. That hashtable is threadsafe (unnecessarily?), and 44% of all time spent in nsHashtable::Get is entering and exiting the lock. Attempting to calculate this all out, I come up with (.02 + .44 * .01) * .15 ==> .366% of execution time manipulating these locks. However, the simple fix is probably to change the flag passed to the hashtable constructor to make it not be threadsafe since the higher level locks are already protecting it.
Based on Warren's analysis, there is a tiny .366% gain (even if we got it down to zero locks) and notable risk. PDT gave this a PDT- for beta1.
Moving all current open XPCOM and XPCOM Registry bugs to rayw since dp is on sabbatical. rayw is now default assignee for these components.
Assigning rest of xpcom stuff to myself (from Ray).
-> kandrot now is a good time for smaller and larger perf wins...
reassign all kandrot xpcom bug.
neeti is working on this as part of her cleanup/performance work in xpcom.
Add myself to the CC list. I'll try to see if I can do some work on it.
This was fixed by bug 96461. nsComponentManager now uses PLDHashtable, instead of the threadsafe version of nsHashtable.