Optimize Array.concat when used on different types of objects

RESOLVED FIXED in Firefox 43

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: bhackett, Assigned: bhackett)

Tracking

Trunk
mozilla43
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 affected, firefox42 affected, firefox43 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

4 years ago
Posted patch patchSplinter Review
Currently we only optimize Array.concat when the 'this' and argument object have the same boxed/unboxed representation.  It would be better if we could optimize concat'ing different kinds of arrays; this affects performance a lot on kraken-stanford-crypto-pbkdf2, where double and int32 unboxed arrays are frequently concatenated.  The attached patch fixes this, and also fixes a couple other performance issues.  The unboxed array analysis is not forced the first time we try to attach a baseline cache for concat and other array natives (on pbkdf2 we would do this too early and end up using an unboxed int32 representation for a group that should really use an unboxed double representation) and the basic concat() native is augmented to use a similar optimized path to the specialized array_concat_dense and avoid some of the perf cost of calling this native.
Attachment #8625004 - Flags: review?(jdemooij)
Assignee

Updated

4 years ago
Assignee: nobody → bhackett1024
Comment on attachment 8625004 [details] [diff] [review]
patch

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

::: js/src/jsarray.cpp
@@ +2565,2 @@
>  DenseElementResult
>  ArrayConcatDenseKernel(JSContext* cx, JSObject* obj1, JSObject* obj2, JSObject* result)

Hm I'm a bit worried about template bloat; this will emit like 36 copies. What's the increase in binary size, debug and opt build?
Assignee

Comment 2

4 years ago
On 10.9 x64 an opt shell is .25% bigger, and a --enable-debug --disable-optimize shell is .23% bigger.
Comment on attachment 8625004 [details] [diff] [review]
patch

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

Sorry for the delay.
Attachment #8625004 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/fdf5862a8c00
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
I backed this out in https://hg.mozilla.org/mozilla-central/rev/d6ea652c5799 to fix bug 1191492.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee

Comment 8

4 years ago
I relanded this patch as is because it looks like another bug caused bug 1191492.
https://hg.mozilla.org/mozilla-central/rev/e671afb66591
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: mozilla42 → mozilla43
Depends on: 1204165
Depends on: 1210596
You need to log in before you can comment on or make changes to this bug.