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

RESOLVED FIXED in Firefox 41

Status

()

defect
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

(Blocks 1 bug)

unspecified
mozilla41
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox41 fixed)

Details

(Whiteboard: [shumway])

Attachments

(2 attachments, 1 obsolete attachment)

6.90 KB, patch
bhackett
: review+
Details | Diff | Splinter Review
504 bytes, text/x-c
Details
(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
Posted file Testcase (obsolete) —
Minimal testcase. Hits both the alloc-slots and realloc-slots cases.
Probably should track this for Shumway.
Flags: needinfo?(cpeterson)
Thanks, Sean.
Blocks: shumway-1.0
Flags: needinfo?(cpeterson)
Whiteboard: [shumway]
(Assignee)

Comment 4

4 years ago
Posted patch PatchSplinter Review
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.
Attachment #8611202 - Flags: review?(bhackett1024)
(Assignee)

Comment 5

4 years ago
Posted file Testcase
Attachment #828595 - Attachment is obsolete: true
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.
Attachment #8611202 - Flags: review?(bhackett1024) → review+
https://hg.mozilla.org/mozilla-central/rev/193cc6c75306
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
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
You need to log in before you can comment on or make changes to this bug.