874678 - B2G: OOM with Google+ caused by JS parsing

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[Gregor Wagner [:gwagner]]

Loading Google+ gives a pretty bad experience. We kill the app after 30 sec to 2 min.
Looking at allocations with instruments on a desktop build shows that part of the problem is JS parsing.
After surfing g+ for around 2 minutes I have 150MB allocated (not JS heap). 30% is allocated by the parser.


Looking at allocations between 1min and 1:30 min I get following numbers:
Bytes Used	Count		Symbol Name
  57.69 MB      90.9%	105533	 	malloc
  38.34 MB      60.4%	5726	 	 js_malloc
  38.34 MB      60.4%	5726	 	  js::detail::BumpChunk::new_(unsigned long)
  38.34 MB      60.4%	5726	 	   js::LifoAlloc::getOrCreateChunk(unsigned long)
  38.34 MB      60.4%	5726	 	    js::LifoAlloc::alloc(unsigned long)
  19.54 MB      30.7%	5001	 	     js::frontend::ParseNodeAllocator::allocNode()
  19.54 MB      30.7%	5001	 	      js::frontend::Parser::allocParseNode(unsigned long)
   8.69 MB      13.6%	278	 	     js::analyze::Bytecode* js::LifoAlloc::new_<js::analyze::Bytecode>()
   3.28 MB       5.1%	89	 	     js::analyze::Bytecode** js::LifoAlloc::newArray<js::analyze::Bytecode*>(unsigned long)
   1.19 MB       1.8%	38	 	     js::types::TypeObjectKey** js::LifoAlloc::newArray<js::types::TypeObjectKey*>(unsigned long)
 640.00 KB       0.9%	20	 	     TypeConstraintSubset* js::LifoAlloc::new_<TypeConstraintSubset, js::types::TypeSet*>(js::types::TypeSet*)
 556.00 KB       0.8%	139	 	     js::frontend::FunctionBox* js::LifoAlloc::new_<js::frontend::FunctionBox, JSContext*, js::frontend::ObjectBox*, JSFunction*, js::frontend::ParseContext*, js::frontend::StrictMode>(JSContext*, js::frontend::ObjectBox*, JSFunction*, js::frontend::ParseContext*, js::frontend::StrictMode)
 512.00 KB       0.7%	16	 	     js::types::Property* js::LifoAlloc::new_<js::types::Property, js::types::Property>(js::types::Property)
 512.00 KB       0.7%	16	 	     js::types::Property** js::LifoAlloc::newArray<js::types::Property*>(unsigned long)
 448.00 KB       0.6%	14	 	     js::types::StackTypeSet* js::LifoAlloc::newArrayUninitialized<js::types::StackTypeSet>(unsigned long)
 448.00 KB       0.6%	14	 	     js::analyze::SSAValue* js::LifoAlloc::newArray<js::analyze::SSAValue>(unsigned long)
 416.00 KB       0.6%	13	 	     js::analyze::SSAUseChain* js::LifoAlloc::newArray<js::analyze::SSAUseChain>(unsigned long)
 416.00 KB       0.6%	13	 	     js::analyze::ScriptAnalysis* js::LifoAlloc::new_<js::analyze::ScriptAnalysis, JSScript*>(JSScript*)
 256.00 KB       0.3%	8	 	     TypeConstraintSubsetBarrier* js::LifoAlloc::new_<TypeConstraintSubsetBarrier, JS::Handle<JSScript*>, unsigned char*, js::types::TypeSet*>(JS::Handle<JSScript*>, unsigned char*, js::types::TypeSet*)
 224.00 KB       0.3%	7	 	     js::types::Property* js::LifoAlloc::new_<js::types::Property, jsid>(jsid)
 192.00 KB       0.2%	6	 	     TypeConstraintFreezeStack* js::LifoAlloc::new_<TypeConstraintFreezeStack, JS::Handle<JSScript*> >(JS::Handle<JSScript*>)
 128.00 KB       0.1%	4	 	     js::analyze::SSAUseChain** js::LifoAlloc::newArray<js::analyze::SSAUseChain*>(unsigned long)
 128.00 KB       0.1%	4	 	     js::mjit::RegisterAllocation* js::LifoAlloc::new_<js::mjit::RegisterAllocation, bool>(bool)
 128.00 KB       0.1%	4	 	     TypeConstraintClearDefiniteSetter* js::LifoAlloc::new_<TypeConstraintClearDefiniteSetter, js::types::TypeObject*>(js::types::TypeObject*)


This is an inverted call tree for all created & still living memory allocations.
The parser has around 20MB allocated and type inference also > 14MB.

Comment 1

[Alex Keybl [:akeybl]]

If v1.0.1 is affected, this isn't a new v1.1 regression. In that case, we'd take a low risk fix but wouldn't block. Adding qawanted to find out if v1.0.1 is impacted.

Comment 2

[Justin Lebar (not reading bugmail)]

I doubt this is a 1.1 regression.

Comment 3

[Jason Smith [:jsmith]]

(In reply to Justin Lebar [:jlebar] from comment #2)
> I doubt this is a 1.1 regression.

Agreed.

(In reply to Alex Keybl [:akeybl] from comment #1)
> If v1.0.1 is affected, this isn't a new v1.1 regression. In that case, we'd
> take a low risk fix but wouldn't block. Adding qawanted to find out if
> v1.0.1 is impacted.

I don't think that should have any effect on the decision to block or not. I think the better question is - would we have blocked if we knew about this on 1.01? Probably, although it's too late for 1.01 at this point.

Comment 4

[Jason Smith [:jsmith]]

Opps, didn't mean to remove qawanted here.

Comment 5

[Jason Smith [:jsmith]]

It's reproducible on 1.01. We're getting a desktop site here also, so in that regard, I'm actually not too concerned now about not blocking on this and instead pushing back on Google to get their mobile site again.

Comment 6

[Jason Smith [:jsmith]]

See bug 878269 for the TE work we'll do here.

Comment 7

[bhavana bajaj [:bajaj]]

Would not block on this given its a TE bug.

Jason,
On a general note are we aware of any other websites that are impacted here ? Do we see this scenario if a mobile website is served as well or does this happen only for websites where mobile content is not being served?

Comment 9

[Eric Rahm [:erahm]]

I can confirm that a logged in G+ instance loads without OOM on a hamachi device. about:memory dump confirms that the page is behaving sanely, even after 5 minutes:

├───2.58 MB (14.52%) -- window-objects
│   ├──2.27 MB (12.81%) -- top(https://plus.google.com/app/basic/stream?cbp=l0y5gg9e1fhj&partnerid=t2searchhp&sview=31&cid=5&soc-app=115&soc-platform=1, id=4)
│   │  ├──2.20 MB (12.38%) -- active/window(https://plus.google.com/app/basic/stream?cbp=l0y5gg9e1fhj&partnerid=t2searchhp&sview=31&cid=5&soc-app=115&soc-platform=1)
│   │  │  ├──0.80 MB (04.54%) ── style-sheets
│   │  │  ├──0.73 MB (04.13%) -- layout
│   │  │  │  ├──0.43 MB (02.41%) ++ (7 tiny)
│   │  │  │  └──0.30 MB (01.71%) ── style-sets
│   │  │  ├──0.42 MB (02.35%) -- dom
│   │  │  │  ├──0.35 MB (01.99%) ── element-nodes
│   │  │  │  └──0.06 MB (00.35%) ++ (4 tiny)
│   │  │  ├──0.24 MB (01.37%) ++ js-compartment(https://plus.google.com/app/basic/stream?cbp=l0y5gg9e1fhj&partnerid=t2searchhp&sview=31&cid=5&soc-app=115&soc-platform=1)
│   │  │  └──0.00 MB (00.01%) ── property-tables
│   │  └──0.07 MB (00.42%) ++ js-zone(0x434f3000)

It would seem that between bug 878269 (loading desktop site on mobile) and bug 876906 (JS parser allocating too much memory) things are running much more smoothly.

Comment 10

[Nicholas Nethercote [inactive]]

Excellent. Just to double-check: what was the js-main-runtime-temporary-peak value in about:memory? That measures the peak size of the parsing data, which can be hard to capture via the other measurements in about:memory because the peak is short-lived.

Also, how was the performance/responsiveness in general -- did it seem ok?

Comment 11

[Eric Rahm [:erahm]]

Seems pretty good:
 0.35 MB ── js-main-runtime-temporary-peak

Performance seemed okay for a hamachi device, a little slow but functional. We're getting the mobile site (rather than the desktop version) now so I'm sure that helps out.

Comment 12

[Nicholas Nethercote [inactive]]

Thanks, Eric. I think we can declare victory here.