Closed Bug 983577 Opened 6 years ago Closed 3 years ago

TypedObject array sequential map significantly slower than Array map

Categories

(Core :: JavaScript Engine, defect)

x86_64
macOS
defect
Not set

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: lth, Unassigned)

References

Details

Attachments

(5 files, 3 obsolete files)

Attached file Simple benchmark (obsolete) —
On a simple benchmark (attach) the untyped 'map' is about 25% faster than the typed 'map' on a 1D TypedObject array.  It does not seem to matter much what the element type of the TypedObject array is - int32, uint32, and float64 all have about the same performance.

(The starting point for this report was a more complicated case where TypedObject map is almost an order of magnitude slower than untyped map; I will report on that when I know more.)
On this more complex benchmark, the untyped Array map is about 77% faster than the TypedObject array map / the typed case takes more than four times as long to run as the untyped case.  Both of them map an array of indices to implement the inner loop of a double loop that represents a matrix as a 1D array.  The mapped function is fairly complicated.
Attached patch typedobject-map.patch (obsolete) — Splinter Review
Preliminary work, but this simple specialization speeds up TypedObject map on the simple benchmark to be within 6% of the Array case, and on the complex benchmark to be within 20%.  (Also contains a specialization for parallel map, which is not used by any of these benchmarks.)
For the complex benchmark we can introduce the function calls from the original code without using their result, to see where the time goes:

- the biggest slowdown comes from the call to inPointer.get() [increase from 930 to 2170ms]
- then the call to outPointer.getOpaqueIf() [increase from 930 to 1770]
- the calls to inPointer.bump() and outPointer.bump() contribute little [increase from 930 to 1100]

All that said, the cumulative slowdown from calling those functions without them having any effect on the functionality does not account for the full slowdown [increase from 930 to 2500, where the full running time is 3200ms].  Consequently there may be some impacts on the efficacy of optimization or code generation from all the method calls.
Assignee: nobody → lhansen
Attachment #8391133 - Attachment is obsolete: true
Attachment #8392138 - Flags: review?(shu)
Attached file Simple benchmark
Cleaned up the simple benchmark.

The patch improves the performance of this benchmark from 75% of the untyped case to 93% of the untyped case.

The patch improves the performance of the complex benchmark from 24% of the untyped case to 83% of the untyped case.

(The remaining performance discrepancy is unexplained.)
Attachment #8391108 - Attachment is obsolete: true
lth: out of curiosity, have you benchmarked TypedArrays (in addition to TypedObject arrays and untyped arrays)?

The reason I ask: There are other tickets that say that TypedArray property get/sets are slow.  See e.g. Bug 930278 or Bug 861974.  There may be performance issues common between TypedArray and TypedObject arrays.
(but I do know that a lot of the TypedObject code used a self-hosted implementation style which may work out better than TypedArrays in some cases, so perhaps I am wrong to draw an analogy connecting performance issues between the two.)
(In reply to Felix S. Klock II [:pnkfelix, :fklock] from comment #8)
> lth: out of curiosity, have you benchmarked TypedArrays (in addition to
> TypedObject arrays and untyped arrays)?

I have not.

> The reason I ask: There are other tickets that say that TypedArray property
> get/sets are slow.  See e.g. Bug 930278 or Bug 861974.  There may be
> performance issues common between TypedArray and TypedObject arrays.

This is useful.  Thanks.
I was recently doing some microbenchmarking between typed objects and normal arrays and I too noticed some performance gaps between the two that I could not explain. The MIR generated in both cases looked pretty much identical -- if anything, the typed objects MIR looked better than the normal array MIR, but the code ran slower. I expect the generated assembly is the culprit.

The fast paths for typed arrays and typed objects both share the same MIR, so issues affecting one could well affect the other.
Comment on attachment 8392138 [details] [diff] [review]
Specialize the depth-one simple types case

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

Looks good to me.

::: js/src/builtin/TypedObject.js
@@ +1290,2 @@
>  
> +  var isSimple1 = depth == 1 && !(inGrainTypeIsComplex || outGrainTypeIsComplex);

Style nit: rename isSimple1 to something slightly more descriptive, like isDepth1Simple

@@ +1360,4 @@
>      return DoMapTypedSeqDepth1();
>    } else {
>      return DoMapTypedSeqDepthN();
>    }

Preexisting nit: else after return
Attachment #8392138 - Flags: review?(shu) → review+
Marking bug as [leave open] until we find the causes of the remaining discrepancy.
Whiteboard: [leave open]
Nits fixed.
Attachment #8392138 - Attachment is obsolete: true
Keywords: checkin-needed
Assignee: lhansen → nobody
Presumably if/when TypedObjects happen we'll have a benchmark suite.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → INCOMPLETE
Whiteboard: [leave open]
You need to log in before you can comment on or make changes to this bug.