Jan de Mooij [:jandem] Assignee
	Description • 7 years ago

For bug 1237504 we want to have reserved slots on proxies. When bz and I were talking about that yesterday, I realized we could get a very easy perf win right now by allocating ProxyValueArray inline in the object. This eliminates a malloc for each non-nursery allocated proxy/wrapper and improves cache locality.

This patch still keeps the ProxyValueArray* pointer. We could remove it now and get rid of the dereference, but since we have JIT code and friend APIs poking at these values it seemed better to do that separately.

	Jan de Mooij [:jandem] Assignee
	Comment 1 • 7 years ago

Oops, we also need to update the ProxyValueArray* pointers in JSObject::swap...

	Jan de Mooij [:jandem] Assignee
	Comment 2 • 7 years ago

Comment on attachment 8860619 [details] [diff] [review]
Patch

Still some orange on Try from JSObject::swap :(

	Jan de Mooij [:jandem] Assignee
	Comment 3 • 7 years ago

To support JSObject::swap, I wonder if we should keep the ProxyValueArray* pointer and make it so that it points either into the object itself (the common case) or to malloc'd data (when swapping with a smaller native object). 

Then bug 1237504 could make the number of Values in ProxyValueArray dynamic (have it depend on the Class).

I think that would work but it's pretty unfortunate that we can't always store these slots inline...

	Jan de Mooij [:jandem] Assignee
	Comment 4 • 7 years ago

This version still allocates ProxyValueArray inline, but JSObject::swap now uses malloc when the object sizes don't match. It seems to work locally: unlike the previous version, a debug build can load Gmail without crashing.

So we can't remove the ProxyValueArray* pointer, but we still eliminate a malloc call for 99% of proxies.

Unfortunately there's an infra issue so Try will have to wait.

	Jan de Mooij [:jandem] Assignee
	Comment 5 • 7 years ago

Here we go. This version looks green on Try so far.

I added some logging and this should eliminate about 20,000 mallocs when starting Firefox. I also checked Dromaeo and there we're talking millions (many non-Nursery-allocated DOM proxies) so hopefully we'll see a perf win there.

	Pulsebot
	Comment 6 • 7 years ago

Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d8027e7b9d7
Allocate ProxyValueArray inline in the object instead of using malloc. r=bhackett

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 7 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/3d8027e7b9d7