Closed Bug 105501 Opened 22 years ago Closed 14 years ago
During startup ns
Memory::Free(...) is being called too much !
nsMemory::Free 52541 calls 0.02ms (without descendents)
And levels of function indirection removed too.
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla0.9.7
Do you have stats that show that this improves speed? Does it really inline this EXPORT'd method? On which platforms? Do we really want to compile in the direct punt to PR_Free? This precludes being able to change the allocator (or do journalling allocator tricks) purely from within xpcom and focused on nsMemory only allocations. Maybe this does not really matter, but evidence that there is a gain would be appreciated.
I am doing timing analysis on this today. Will report timing. How would I prove that inline is happening ? Look at assembly ?
This sounds alot like http://bugzilla.mozilla.org/show_bug.cgi?id=75620.
yes - this patch does inline. I looked at assembly on linux. And my testing is showing a 4.8% improvement in startup. I am going to retest again. In fact, folks are suggesting I should go directly to free() instead of the indirection to PR_Free. I am going to test that out.
4.8% was definitely too good to be true. I used the wrong build. :-( The improvement is about 0.4% of startup. At this granularity, error in measurement is high. The good news is it does look like an improvement. :-) Although this is small, I vote for taking it.
*** This bug has been marked as a duplicate of 75620 ***
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → DUPLICATE
If there *really* are 52541 calls to nsMemory::Free(...) at startup, then the 'fix' is *not* to make Free(...) faster !!! The issue is that we are creating too many short-lived objects on the heap. Think about it... Startup *should not* create lots of short-lived objects!! So, after some Q&D hacks to nsMemory here is some data about heap utilization in both mozilla and mfcEmbed (the embedding case) All of this data was collected from optimized builds (starting up with about:blank) that did not *need* to perform any component registration... Mozilla: ======== 1. After calling SetupRegistry(...) Total Allocations: 469 Live Allocations: 218 Heap utilization: ~38% 2. Before calling AppShell::Run(...) Total Allocations: 6362 Live Allocations: 1832 Heap utilization: ~29% NULL frees: 79 (~1.7%) 3. After 10,000 Allocations: Total Allocations: 10000 Live Allocations: 2725 Heap utilization: ~27% NULL frees: 139 (~1.9%) 4. After 20,000 Allocations: Total Allocations: 20000 Live Allocations: 4661 Heap utilization: ~23% NULL frees: 217 (~1.4%) 5. After 40,000 Allocations: Total Allocations: 40000 Live Allocations: 8170 Heap utilization: ~20% NULL frees: 641 (~2.0%) MFCEmbed: ========= 1. After calling SetupRegistry(...): Total Allocations: 546 Live Allocations: 214 Heap utilization: ~39% 2. Before calling OnNewBrowser(...): Total Allocations: 7359 Live Allocations: 858 Heap utilization: ~12% NULL frees: 4 (~0%) 3. After 10,000 Allocations: Total Allocations: 10000 Live Allocations: 1397 Heap utilization: ~14% NULL frees: 35 (~0.4%) 4. At shutdown: Total Allocations: 14750 NULL frees: 155
From the data above, it is clear that we are creating too many short lived objects!! It is interesting to note that in the embedding case, while we create many fewer objects our overall heap utilization is almost HALF of what it is in mozilla !!! This seems VERY surprising and BAD!! It is also worth noting that in mozilla, it appears that we call nsMemory::Free(...) about 1% of the time passing NULL ! These cases could easily be short-circuited :-) So, from this data, if work is being done to 'speed up' nsIMemory then PLEASE focus on how to reduce the number of short lived objects being created during initialization!! This will 'speed up' nsMemory::Free(...) a whole bunch! -- rick
One last thing that i forgot to mention was that our usage of nsMemory::Alloc and nsMemory::Free is *not* symetrical!! We end up calling nsMemory::Free(...) with blocks that were *not* returned from nsMemory::Alloc(...) !! This does *not* cause heap corruption (only) because all of our 'allocators' ultimately call down into malloc. But if we ever *do* create custom (non-malloc based) allocators then this issue MUST be addressed. -- rick
i'm reopening this bug to track the real issue which is that during startup we are making too many shortlived memory allocations. -- rick
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Summary: nsMemory::Free needs to be inlined → During startup nsMemory::Free(...) is being called too much !
0.9.6 is long gone. -> 0.9.7
Target Milestone: mozilla0.9.6 → mozilla0.9.7
stacks for short-lived objects? spacetrace can find them. /be
when you get around to using spacetrace, you'll find in the options or command line switches the ability to exclude/include allocations that live a particular length of time. a lifespan under 1 second looks to be ideal for what you're after. what you'll still be missing if you run today is the ability to sort by number of allocations performed. i am in the process of adding this for dp, so the reality of reporting exactly what you need is not that far away.
I agree with Rick. The FIX is to reduce the number of calls. spacetrace has all data one needs to look at their module and see the number, amount and time spend on allocations. But this is far from being a person effort. Maybe I should file a couple of bugs and make this one a tracking bug.
Status: REOPENED → ASSIGNED
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Moving past 1.0 I dont believe we can significantly impact this before 1.0 We are however going to reduce the number of allocation requests and hence the free requests in our effort to reduce Vm usage and fragmentation.
Target Milestone: mozilla0.9.9 → mozilla1.0.1
So this bug should be titled 'Startup creates too many shortlived memory allocations' ?
Should mlk be added to keywords?
Assignee: dp → nobody
Status: ASSIGNED → NEW
QA Contact: scc → xpcom
is this bug valid anymore?
closing - most recent substantive comments were 7 years ago. -> INCOMPLETE
Status: NEW → RESOLVED
Closed: 22 years ago → 14 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.