935932 - IonMonkey: add-property stub: support dynamic slot (re)allocation

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Jan de Mooij [:jandem]]

We don't optimize the case where we have to allocate or reallocate dynamic slots. TypeScript (bug 935929) hits this a lot. It should be possible to callWithABI a function to do this and with GGC we can inline it completely if the object is still in the nursery.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Testcase

Minimal testcase. Hits both the alloc-slots and realloc-slots cases.

Comment 2

[Sean Stangl [:sstangl]]

Probably should track this for Shumway.

Comment 3

[Chris Peterson [:cpeterson]]

Thanks, Sean.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Patch

Time to fix this; it hits a lot on various frameworks/websites.

The patch improves a micro-benchmark from 530 to 90 ms.

Later we can do nursery reallocation of slots inline, but that's a lot more complicated and this patch fixes the worst perf cliff.

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Testcase

Comment 6

[Brian Hackett [Laid off!]]

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

Review of attachment 8611202 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/vm/NativeObject.h
@@ +612,5 @@
>      bool growSlots(ExclusiveContext* cx, uint32_t oldCount, uint32_t newCount);
>      void shrinkSlots(ExclusiveContext* cx, uint32_t oldCount, uint32_t newCount);
>  
> +    /* This method is static because it's called from JIT code. */
> +    static bool growSlots(ExclusiveContext* cx, NativeObject* obj, uint32_t newCount);

Maybe rename this (growSlotsStatic) so it's less likely to be confused with the other |growSlots| and called like an instance method.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/193cc6c75306

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/193cc6c75306

Comment 9

[Hannes Verschore [:h4writer]]

I assume this caused by this patch. Nice!

A regression/improvement was reported on AWFY:
- slave: Mac OS X 10.10 (Mac Pro, browser)
- mode: Ion

Regression(s)/Improvement(s):
- speedometer: 2.4% (improvement)
- jetstream: box2d: 2.08% (improvement)
- assorteddom: misc-window-getname-performance-getter: -3.08% (improvement)

Recorded range:
- http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=706666a0ad01&tochange=b2c2577ac49c

More details: http://arewefastyet.com/regressions/#/regression/1694791