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)

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: 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?
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
Closed: 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 → ---
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
Closed: 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.