115973 - 1MB churned by nsXPCWrappedJSClass::CallMethod

Status: RESOLVED DUPLICATE of bug 408113

(Core :: JavaScript Engine, defect, P2)

Description

[Suresh Duddi (gone)]

We seem to allocate js_AllocStack() which causes a 8224 allocation and this
lives only for 0.005 sec.

Overall memory churned: 37 * 8224 = 304288

Can we allocate once and reuse or maybe even use the stack

Comment 1

[Suresh Duddi (gone)]

Looks like this happens more than 100 times churning about 1MB

Comment 2

[Boris Zbarsky [:bzbarsky]]

This is 100 times over the course of startup?

Comment 3

[John Bandhauer]

We pretty much have to use js_AllocStack in order to do JS calls. Perhaps the JS
engine is too aggresive in reclaiming this space?

Comment 4

[Brendan Eich [:brendan]]

The stack is allocated from an arena pool, cx->stackPool, so there should be
good reuse.  dp, can you show the stack backtrace for a typical (or all) free
call(s) that gives back to the malloc heap the memory allocated to cx->stackPool
by nsXPCWrappedJSClass::CallMethod's js_AllocStack?

/be

Comment 5

[Suresh Duddi (gone)]

Attachment: 37 calls to js_AllocStack

This is from spacetrace.

Comment 6

[Suresh Duddi (gone)]

Boris, yes this is 100+ times for starting up the browser with default profile
and mozilla.org start page

Comment 7

[Brendan Eich [:brendan]]

dp, I need the matching free stacks -- not all js_AllocStack calls should
malloc, either, but those that do should not be matched by a free on the
matching js_FreeStack call -- arena pools hang onto arenas until they're
"Finished" by a call to JS_GC on that cx.  IOW, do the free calls that match
these mallocs come from JS_GC, or from somewhere else?

/be

Comment 8

[Suresh Duddi (gone)]

Space trace shows only the stack traces for allocation.

Garrett is there any way you can harness from the trace-malloc log, the matching
free call.

Comment 9

[Garrett Arch Blythe]

A callsite is recorded in the trace-malloc log, but it is that of the original
malloc callsite if I recall.

you'll need to modify xpcom/base/nsTraceMalloc.c to record the correct free
callstack.

this is a little more involved than simply changed spacetrace; there may even be
dependencies on the callstack of the free being the same hash item as the
callstack of the malloc in the tool, but I would have to check or wait till it
broke and then fix it.

Comment 10

[David Bradley]

Should this be assigned to the JS Engine component and team member?

Comment 11

[Brendan Eich [:brendan]]

Taking.

/be

Comment 12

[Cathleen]

I have a spacetrace log for compose window (mozilla.exe -compose), with 5 lines
of text typed in it.  see screen shot later.

Data on my machine showed that we chug through 11MB (11,702,752 Bytes) of heap
by js_AllocRawStack

Comment 13

[Cathleen]

Attachment: composed msg (screen shot)

Comment 14

[Brendan Eich [:brendan]]

Missed 0.9.8, but thanks for the space-trace -- I'll look tonight.  Just
cleaning my buglist now.

/be

Comment 15

[Simon Fraser [no longer active]]

All platforms.

Comment 16

[Brendan Eich [:brendan]]

malloc is not inherently evil, but the churn here seems high.  Probably our 8192
byte actual param (http://lxr.mozilla.org/mozilla/search?string=gStackSize)
passed to JS_NewContext by the DOM is too large.  I don't have time to look at
this right now, but I'll try to find some soon.  This may slip into 1.0.

/be

Comment 17

[Brendan Eich [:brendan]]

This can't block 0.9.9.  I'll look into dropping gStackSize for 1.0, if not
inventing an adaptive size scheme.

/be

Comment 18

[Cathleen]

reduce churn --> reduce malloc overhead --> reduce data footprint
nsbeta1  :-)

Comment 19

[Brendan Eich [:brendan]]

shaver, you want this one?  If not, can you lay any utilization-measuring patch
you may have on us here?

/be

Comment 20

[Brendan Eich [:brendan]]

Moving out a bit.  The js1.5 release (after an rc5 release candidate) should
come off off the 1.0 branch on or before 1.0.2 freezing, I say.  The trunk and
branch should be in sync for js/src/js*.[chmt]* and build files.

/be

Comment 21

[Brendan Eich [:brendan]]

Fixing TM.

/be

Comment 22

[Samir Gehani]

adt: nsbeta1-

Comment 23

[Brendan Eich [:brendan]]

This needs to be re-measured and stuff.

/be

Comment 24

[Worcester12345]

From log: "[e-mail] 2004-04-19 22:25 PDT Target Milestone  
mozilla1.7alpha Future"

1.7 is now past alpha. "The future" has arrived. ?ing it to put on radar.(In
reply to comment #23)
> This needs to be re-measured and stuff.
> 
> /be
Was it ever re-measured "and stuff"?


From log: "[e-mail] 2004-04-19 22:25 PDT Target Milestone  
mozilla1.7alpha Future"

1.7 is now past alpha. "The future" has arrived. ?ing it to put on radar.

Comment 25

[Boris Zbarsky [:bzbarsky]]

> Was it ever re-measured "and stuff"?

No.  If it had been, the results would be in the bug.  Feel free to remeasure
and post the results here.  Otherwise, please stop spamming bugs with uses comments.

Comment 26

[Brendan Eich [:brendan]]

Before nominating this as blocking an alpha, which is unlikely for a non-crash,
non-dataloss bug, someone should measure.

What bz said, too.

/be

Comment 27

[Worcester12345]

I meant to change the "target milestone", but did a blocking flag by mistake. At
least I did the latest. Just doing some bug QA work while I had a second. Sorry
for any inconvenience.

Comment 28

[Brendan Eich [:brendan]]

Sasquatch, please don't change Target Milestone, that belongs to the assignee or
a designated helper.  Have you read the bugzilla docs and etiquette guides?

/be

Comment 29

[Worcester12345]

(In reply to comment #28)
> Sasquatch, please don't change Target Milestone, that belongs to the assignee or
> a designated helper.  Have you read the bugzilla docs and etiquette guides?
> 
> /be

All is OK, it wouldn't allow that either.

So, how else would one get one of these old bugs out of mothballs and either put
to rest or worked on? Just trying to contribute by doing some bug QA here.

Comment 30

[Brendan Eich [:brendan]]

Sasquatch: it's unfortunately hard to test these bugs without added
instrumentation, and without using tools such as spacetrace.  Maybe that's a
tool you could use, though -- IIRC it requires 1GB RAM to run well on the same
machine!

/be

Comment 31

[Brendan Eich [:brendan]]

Disowning formally, obvious I've been a deadbeat dad to this bug. Someone else will have to take up the cause. For Mozilla 2 we will use Tamarin underneath of SpiderMonkey, and avoid XPConnected DOM altogether.

/be

Comment 32

[Brendan Eich [:brendan]]

Could dup forward to bug 408113, or dup that against this. Crowder, are you able to make time to implemnet the idea I sketched there? It would be good to get the simplest patch along those lines in soon.

/be

Comment 33

[Brian Crowder]

Duping forward, since that bug has more love in the way of patches