Closed
Bug 978077
Opened 11 years ago
Closed 11 years ago
2D TypedObject arrays much slower than 1D arrays due to intermediate allocations
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: lth, Assigned: h4writer)
References
Details
(Keywords: perf, Whiteboard: [js:p2])
Attachments
(4 files, 2 obsolete files)
A program that convolves an image is an order of magnitude faster if it uses 1D TypedObject arrays to simulate a 2D array than if it uses 2D TypedObject arrays (2.6s vs 30s for 100 iterations on my system). The reason for the discrepancy is (likely) boxing of intermediate results: when a[h][w] is evaluated, an intermediate object is created for a[h].
JS shell, release build, 64-bit.
Numbers to back this up: Running with MOZ_GCTIMER=file.txt, the log reveals about 9GB of allocation for the 100 iterations. The image is 640x600, and each array element is evaluated nine times. This works out to about 39 bytes per element, which is a credible object size for the intermediate result (four or five words).
Reporter | ||
Updated•11 years ago
|
Blocks: harmony:typedobjects
Reporter | ||
Comment 1•11 years ago
|
||
Reporter | ||
Comment 2•11 years ago
|
||
Reporter | ||
Comment 3•11 years ago
|
||
Reporter | ||
Comment 4•11 years ago
|
||
From nmatsakis on IRC:
they used to be fast ;) but I think that as part of bug 898356 I may have introduced a perf hit
in particular, we used to optimize away all intermediate objects,
but I think because we now check for nullability,
that is, neutering of a buffer due to sending to another thread,
we introduce fallibility,
and the temporary wrappers get introduced for bailout recovery
fixing this is a bit tricky but doable
(to clarify: when you write x[1][2], there is an intermediate object introduced for x[1])
(we used to optimize this object away entirely, but I suspect we no longer do)
Updated•11 years ago
|
Reporter | ||
Comment 5•11 years ago
|
||
Observation: when the call to Math.max() is replaced by calls to a straightforward local max() function, the running time drops from 29s to 1.5s. It looks like perhaps the call to Math.max() causes some sort of deoptimization / pessimization that forces an intermediate allocation. Without the call to Math.max, there is hardly any allocation/GC.
Component: JavaScript Engine → JavaScript Engine: JIT
Reporter | ||
Comment 6•11 years ago
|
||
Re the last comment, the performance cliff is particularly ironic because by the time Math.max() is called, all the intermediate objects but one will be dead. Thus if we're able to optimize out intermediate objects for the max() case we ought - somewhat reasonably - to be able to do almost as well for the Math.max() case.
Reporter | ||
Comment 7•11 years ago
|
||
Attachment #8383665 -
Attachment is obsolete: true
Reporter | ||
Comment 8•11 years ago
|
||
Attachment #8383666 -
Attachment is obsolete: true
Reporter | ||
Comment 9•11 years ago
|
||
Perhaps related, though probably not: in the 1D case, using Math.max() instead of the hand-written max() doubles the running time of the benchmark.
Assignee | ||
Comment 10•11 years ago
|
||
Did some detective work. And it is indeed correct that Math.max() will be much slower than using the self-made max function. The reason is that the input types to Math.max() are integer and double, but we see only integer as output to Math.max(). (e.g. Math.max(0.4, 100)). In that case we don't shortcut Math.max to use MMinMax and use a full call, which is much slower than doing the self-made max function that gets inlined. (This is due to former TI restrictions).
I think we now can just remove that extra condition and always inline. I already tried this and noticed something odd afterwards. So need to investigate further. I will try to come back on this towards the end of the week.
Assignee | ||
Comment 11•11 years ago
|
||
Like said before, we don't need to update TI accordingly anymore. So we can inline math.max when outputtype is integer and there is an input type double. By default generate a double minmax, since using an integer minmax will add ToInteger to the double argument, letting us bail.
Without patch, Math.max: infinity ms
Without patch, max5(): 990ms
With patch, Math.max: 604ms
Assignee: nobody → hv1989
Attachment #8392877 -
Flags: review?(evilpies)
Comment 12•11 years ago
|
||
Sorry, I don't really understand what changed with TI so this is allowed. Can you please ask somebody else. :(
Assignee | ||
Updated•11 years ago
|
Attachment #8392877 -
Flags: review?(evilpies) → review?(jdemooij)
Updated•11 years ago
|
Attachment #8392877 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 13•11 years ago
|
||
Comment 14•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•