412868 - Excessive number of js_NewScope calls while running sunspider test

Status: RESOLVED DUPLICATE of bug 558451

(Core :: JavaScript Engine, defect, P2)

Description

[Stuart Parmenter]

Running the sunspider test I see _a lot_ of new scopes being created.  More than I feel is reasonable:

              libmemory.dylib`zone_malloc+0x11
              libSystem.B.dylib`malloc_zone_malloc+0x51
              libSystem.B.dylib`malloc+0x37
              libmozjs.dylib`JS_malloc+0x33
              libmozjs.dylib`js_NewScope+0x1b
              libmozjs.dylib`js_GetMutableScope+0x42
              libmozjs.dylib`js_DefineNativeProperty+0x1d4
              libmozjs.dylib`js_DefineProperty+0x52
              libmozjs.dylib`str_resolve+0x10e
              libmozjs.dylib`js_LookupPropertyWithFlags+0x2bd
              libmozjs.dylib`js_GetProperty+0xf2
              libmozjs.dylib`js_Interpret+0x5e91
              libmozjs.dylib`js_Execute+0x292
              libmozjs.dylib`JS_EvaluateUCScriptForPrincipals+0x82
              XUL`nsJSContext::EvaluateString(nsAString_internal const&, void*, nsIPrincipal*, char const*, unsigned int, unsigned int, nsAString_internal*, int*)+0x2e1

           value  ------------- Distribution ------------- count    
              16 |                                         0        
              32 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ 506040   
              64 |                                         0        

thats right, over 500000 mallocs! wee!

If you need a bigger stack let me know.

Comment 1

[Stuart Parmenter]

additional stacks of horror:

              libmemory.dylib`zone_malloc+0x11
              libSystem.B.dylib`malloc_zone_malloc+0x51
              libSystem.B.dylib`malloc+0x37
              libmozjs.dylib`JS_malloc+0x33
              libmozjs.dylib`js_NewScope+0x1b
              libmozjs.dylib`js_GetMutableScope+0x42
              libmozjs.dylib`js_DefineNativeProperty+0x1d4
              libmozjs.dylib`call_resolve+0x15a
              libmozjs.dylib`js_LookupPropertyWithFlags+0x2bd
              libmozjs.dylib`js_LookupProperty+0x35
              libmozjs.dylib`js_FindProperty+0x4f
              libmozjs.dylib`js_Interpret+0x6b54
              libmozjs.dylib`js_Execute+0x292
              libmozjs.dylib`obj_eval+0x4d1
              libmozjs.dylib`js_Invoke+0x877

           value  ------------- Distribution ------------- count    
              16 |                                         0        
              32 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ 4552     
              64 |                                         0        

              libmemory.dylib`zone_malloc+0x11
              libSystem.B.dylib`malloc_zone_malloc+0x51
              libSystem.B.dylib`malloc+0x37
              libmozjs.dylib`JS_malloc+0x33
              libmozjs.dylib`js_NewScope+0x1b
              libmozjs.dylib`js_GetMutableScope+0x42
              libmozjs.dylib`js_SetProperty+0x328
              libmozjs.dylib`js_Interpret+0x92a3
              libmozjs.dylib`js_Execute+0x292
              libmozjs.dylib`obj_eval+0x4d1
              libmozjs.dylib`js_Invoke+0x877
              libmozjs.dylib`js_Interpret+0x684d
              libmozjs.dylib`js_Execute+0x292
              libmozjs.dylib`JS_EvaluateUCScriptForPrincipals+0x82
              XUL`nsJSContext::EvaluateString(nsAString_internal const&, void*, nsIPrincipal*, char const*, unsigned int, unsigned int, nsAString_internal*, int*)+0x2e1

           value  ------------- Distribution ------------- count    
              16 |                                         0        
              32 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ 12531    
              64 |                                         0        

              libmemory.dylib`zone_malloc+0x11
              libSystem.B.dylib`malloc_zone_malloc+0x51
              libSystem.B.dylib`malloc+0x37
              libmozjs.dylib`JS_malloc+0x33
              libmozjs.dylib`js_NewScope+0x1b
              libmozjs.dylib`js_GetMutableScope+0x42
              libmozjs.dylib`js_SetProperty+0x328
              libmozjs.dylib`SetArrayElement+0x3a
              libmozjs.dylib`InitArrayElements+0x45
              libmozjs.dylib`InitArrayObject+0x8e
              libmozjs.dylib`Array+0xb4
              libmozjs.dylib`js_Invoke+0x877
              libmozjs.dylib`js_InvokeConstructor+0x17c
              libmozjs.dylib`js_Interpret+0x4514
              libmozjs.dylib`js_Execute+0x292

           value  ------------- Distribution ------------- count    
              16 |                                         0        
              32 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ 13517    
              64 |                                         0        

              libmemory.dylib`zone_malloc+0x11
              libSystem.B.dylib`malloc_zone_malloc+0x51
              libSystem.B.dylib`malloc+0x37
              libmozjs.dylib`JS_malloc+0x33
              libmozjs.dylib`js_NewScope+0x1b
              libmozjs.dylib`js_GetMutableScope+0x42
              libmozjs.dylib`js_SetProperty+0x328
              libmozjs.dylib`js_Interpret+0x60c8
              libmozjs.dylib`js_Execute+0x292
              libmozjs.dylib`JS_EvaluateUCScriptForPrincipals+0x82
              XUL`nsJSContext::EvaluateString(nsAString_internal const&, void*, nsIPrincipal*, char const*, unsigned int, unsigned int, nsAString_internal*, int*)+0x2e1
              XUL`nsScriptLoader::EvaluateScript(nsScriptLoadRequest*, nsString const&)+0x228
              XUL`nsScriptLoader::ProcessRequest(nsScriptLoadRequest*)+0x95
              XUL`nsScriptLoader::ProcessScriptElement(nsIScriptElement*)+0x124c
              XUL`nsScriptElement::MaybeProcessScript()+0xf8

           value  ------------- Distribution ------------- count    
              16 |                                         0        
              32 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ 82288    
              64 |                                         0        


              libmemory.dylib`zone_malloc+0x11
              libSystem.B.dylib`malloc_zone_malloc+0x51
              libSystem.B.dylib`malloc+0x37
              libmozjs.dylib`JS_malloc+0x33
              libmozjs.dylib`js_NewScope+0x1b
              libmozjs.dylib`js_GetMutableScope+0x42
              libmozjs.dylib`js_SetProperty+0x328
              libmozjs.dylib`js_Interpret+0x5d83
              libmozjs.dylib`js_Invoke+0x8dc
              libmozjs.dylib`js_InvokeConstructor+0x17c
              libmozjs.dylib`js_Interpret+0x4514
              libmozjs.dylib`js_Execute+0x292
              libmozjs.dylib`JS_EvaluateUCScriptForPrincipals+0x82
              XUL`nsJSContext::EvaluateString(nsAString_internal const&, void*, nsIPrincipal*, char const*, unsigned int, unsigned int, nsAString_internal*, int*)+0x2e1
              XUL`nsScriptLoader::EvaluateScript(nsScriptLoadRequest*, nsString const&)+0x228

           value  ------------- Distribution ------------- count    
              16 |                                         0        
              32 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ 161431   
              64 |                                         0        


              libmemory.dylib`zone_malloc+0x11
              libSystem.B.dylib`malloc_zone_malloc+0x51
              libSystem.B.dylib`malloc+0x37
              libmozjs.dylib`JS_malloc+0x33
              libmozjs.dylib`js_NewScope+0x1b
              libmozjs.dylib`js_GetMutableScope+0x42
              libmozjs.dylib`js_SetProperty+0x328
              libmozjs.dylib`js_Interpret+0x92a3
              libmozjs.dylib`js_Execute+0x292
              libmozjs.dylib`JS_EvaluateUCScriptForPrincipals+0x82
              XUL`nsJSContext::EvaluateString(nsAString_internal const&, void*, nsIPrincipal*, char const*, unsigned int, unsigned int, nsAString_internal*, int*)+0x2e1
              XUL`nsScriptLoader::EvaluateScript(nsScriptLoadRequest*, nsString const&)+0x228
              XUL`nsScriptLoader::ProcessRequest(nsScriptLoadRequest*)+0x95
              XUL`nsScriptLoader::ProcessScriptElement(nsIScriptElement*)+0x124c
              XUL`nsScriptElement::MaybeProcessScript()+0xf8

           value  ------------- Distribution ------------- count    
              16 |                                         0        
              32 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ 175412   
              64 |                                         0        

Comment 2

[Brendan Eich [:brendan]]

(In reply to comment #0)
> Running the sunspider test I see _a lot_ of new scopes being created.  More
> than I feel is reasonable:

What is this, Oprah? You "feel"?! :-P

How many mutated objects does the benchmark make? Some analysis of the workload should precede judgment of implemetation. Currently, each mutated native object in SpiderMonkey gets its own JSScope (which is independently malloc'ed).

Since this bug is looking "meta", a general comment:

Small mallocs are not a problem _per se_, unless the malloc implementation does badly for some reason. We're taking chances right now reducing malloc call counts by using arenas, which can lead to bad worst-case fragmentation (hoarding of largely unused arenas in pools no longer subject to allocation requests).

The full mutated object for closure environments (Call objects) shown in some of your stacks is probably best treated in bug 411575.

Probably the best way to go here is to block on bug 411575 and others, filed based on workload analysis and ideas about how to reduce costs associated with mutable objects.

/be

Comment 3

[Stuart Parmenter]

There may be nothing we can do here, but the numbers far far far exceed the number of allocations of anything else I've seen.  I'm not super worried about these for fragmentation but reducing some % of 750000+ allocations should speed us up.

Figured it was worth getting a bug on file.  

Comment 4

[Brian Crowder]

Attachment: keep scopes until we can gc them all in one go

This idea is the same as the idea for bug 417131.  In my sunspider testing, it doesn't save much, if anything.  Just throwing up the patch, as it is.

Comment 5

[Brian Crowder]

Another idea might be to pool small "clumps" of scopes together in contiguous allocations.

Comment 6

[Igor Bukanov]

(In reply to comment #5)
> Another idea might be to pool small "clumps" of scopes together in contiguous
> allocations.
> 

Clumps may improve the locality as well. The only complication AFAICS is know a position in clump that slot takes to be able to release it, but JSScope.spare can be used for that.  

Comment 7

[Brian Crowder]

Recommend reducing this from blocking to wanted.  I don't think these scope allocations are very costly relative to what's required of the code here.

Comment 8

[Stuart Parmenter]

crowder: yeah, thats fine.  Maybe spend a little more time and see if you can get any wins here otherwise don't worry about it.

Comment 9

[Brendan Eich [:brendan]]

Comment on attachment 303601 [details] [diff] [review]
keep scopes until we can gc them all in one go

This will hoard an unbounded number of scopes in one context -- consider a gmail session in a tab that allocates a bunch and then is left idle for a long while, as the user moves to another tag and interacts with something that allocates many mutated objects.

Suggest you count the number of mutated objects created by a full SunSpider run. I thikn it will be the same as the number of scopes, or almost the same (there may be a case where SpiderMonkey creates an empty scope to simplify things, e.g. when setting __proto__).

Then to reduce malloc load we could try to GC allocate scopes (just chunking them is bad again as a single live scope could tie up a chunk, and we do not want to reinvent the GC heap just for this chunking system).

/be