687046 - Performance tests for ByteArray

Status: RESOLVED WONTFIX

(Tamarin Graveyard :: Library, defect, P3)

Description

[Lars T Hansen]

We need microbenchmarks and canaries for the ByteArray performance work.

Comment 1

[Lars T Hansen]

Attachment: Patch 1: the bulk of the microbenchmarks

This set is missing a few things, notably these:

- writeUTF / writeUTFBytes
- write* that extend the buffer
- readUTF / readUTFBytes / writeUTF / writeUTFBytes with non-7-bit data

Also, for a given set of tests bytearray-{read,write}-X-*.as the workload is intended to be constant, but that does not hold at this time for readUTF / readUTFBytes.  I may or may not wish to fix that, by and by; for now it's most important just to measure *something*.

Comment 2

[Felix S. Klock II [:pnkfelix, :fklock]]

Comment on attachment 560912 [details] [diff] [review]
Patch 1: the bulk of the microbenchmarks

Yep, its a start.  R+

The main suggestions that came to my mind were to add additional benchmarks timing reads of 'position' or 'bytesAvailable' interspersed with reads/writes to the ByteArray.  But such benchmarks can be added later, if need be; no reason to hold up this patch.

Comment 3

[Tamarin Bot]

changeset: 6592:a4f648fa8281
user:      Lars T Hansen <[e-mail]>
summary:   For 687046 - Performance tests for ByteArray: first batch (r=fklockii)

http://hg.mozilla.org/tamarin-redux/rev/a4f648fa8281

Comment 4

[Lars T Hansen]

First batch of benchmarks landed in Brannan, so targeting.

Comment 5

[Lars T Hansen]

Keeping this open: more benchmarks are coming.

Comment 6

[Lars T Hansen]

Attachment: Canary for ByteArray: vector->bytevector, bytevector->vector

A complaint I've heard (from Grayson Lang) is that his code spends too much time grubbing data out of a ByteArray and copying it into a Vector, and presumably there will also be some conversion the other direction.  These small programs test that use case.

Three cases in each direction: little-endian aligned, big-endian aligned, and big-endian unaligned.  The last is probably the worst case universally because no big-endian architectures support unaligned loads.

FWIW: On MacPro / 10.6 / 2.93GHz Xeon / 32-bit Release, these speed up from about 260 it/sec with TR-6006 to to about 800 it/sec with TR-tip + bytevector optimizations (750 it/sec for big-endian).  800/260=3.06, not bad for code that somebody apparently cares about.

Comment 7

[Lars T Hansen]

Attachment: Microbenchmarks for ByteArray vs MOPs vs Vector (int loop deathmatch benchmarks)

There's more information about what these benchmarks are for in the README enclosed in the patch.

I hesitate a little to put these up for review since they have to be run from the enclosed Makefile and not from the normal performance harness, but they are self-contained and useful and I don't want them to be lost.  The reason they can't be run from the normal harness is that the harness can't handle .abs files, nor can it handle precompiled .abc files.  (I will file a bug report for that.)

Comment 8

[Lars T Hansen]

Inability of test/performance/runtests.py to handle .abs files filed as bug #692718.

Comment 9

[Lars T Hansen]

Attachment: Patch 2: more microbenchmarks

Adds ByteArray microbenchmarks: write "position" (important for random access use case), writeUTF, writeUTFBytes.

Comment 10

[Felix S. Klock II [:pnkfelix, :fklock]]

Comment on attachment 565168 [details] [diff] [review]
Canary for ByteArray: vector->bytevector, bytevector->vector

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

R+

Overall question: Do we need to consider tests of aligned non-zero starting positions?  All of the aligned tests here start at zero and I'm not yet convinced that's the most representative case to consider.

::: test/performance/misc/bytearray-to-vector-worst.as
@@ +40,5 @@
> +
> +// Probable worst case - big-endian systems (SPARC, PPC) tend to require alignment,
> +// so even on those systems we'll be reading byte-at-a-time.
> +
> +var DESC = "Read uints from aligned ByteArray into unaligned Vector.<uint>, big-endian";

I don't know what an unaligned Vector.<uint> means, and I cannot tell what in the code below would make these vectors unaligned or otherwise special.

From the test itself, it looks more like we're reading from an unaligned starting position in the ByteArray, which sounds like the opposite of what the description above says.

Comment 11

[Felix S. Klock II [:pnkfelix, :fklock]]

Comment on attachment 565315 [details] [diff] [review]
Microbenchmarks for ByteArray vs MOPs vs Vector (int loop deathmatch benchmarks)

R+; I read the README and skimmed the as3 code, but am rubber-stamping the abs code.

Comment 12

[Felix S. Klock II [:pnkfelix, :fklock]]

Comment on attachment 565458 [details] [diff] [review]
Patch 2: more microbenchmarks

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

R+

::: test/performance/asmicro/bytearray-write-utf-1.as
@@ +38,5 @@
> + * ***** END LICENSE BLOCK ***** */
> +
> +import flash.utils.ByteArray;
> +
> +var DESC = "Write short string as length-prefixed UTF to pre-sized ByteArray";

consider expanding to
  "Write short string as 2-byte-length-prefixed UTF to pre-sized ByteArray"

(or not, if you prefer; I'm just trying to save others from the head-scratching I had for a bit looking at the 2's below.  Though I probably had missed the "length-prefixed" in the first place, so maybe it wouldn't have helped anything.)

Comment 13

[Lars T Hansen]

(In reply to Felix S Klock II from comment #10)

> Overall question: Do we need to consider tests of aligned non-zero starting
> positions?  All of the aligned tests here start at zero and I'm not yet
> convinced that's the most representative case to consider.

Given the structure of the code and considering most reasonable implementations I don't think it matters (apart from cache effects).

> 
> ::: test/performance/misc/bytearray-to-vector-worst.as
> @@ +40,5 @@
> > +
> > +// Probable worst case - big-endian systems (SPARC, PPC) tend to require alignment,
> > +// so even on those systems we'll be reading byte-at-a-time.
> > +
> > +var DESC = "Read uints from aligned ByteArray into unaligned Vector.<uint>, big-endian";
> 
> I don't know what an unaligned Vector.<uint> means, and I cannot tell what
> in the code below would make these vectors unaligned or otherwise special.
> 
> From the test itself, it looks more like we're reading from an unaligned
> starting position in the ByteArray, which sounds like the opposite of what
> the description above says.

Thanks, I'll fix that - it's the ByteArray that's unaligned, of course.

Comment 14

[Tamarin Bot]

changeset: 6651:00990d795cd1
user:      Lars T Hansen <[e-mail]>
summary:   For 687046 - Performance tests for ByteArray: Microbenchmarks for ByteArray vs MOPs vs Vector (int loop deathmatch benchmarks) (r=fklockii)

http://hg.mozilla.org/tamarin-redux/rev/00990d795cd1

Comment 15

[Tamarin Bot]

changeset: 6652:fba3ca915295
user:      Lars T Hansen <[e-mail]>
summary:   For 687046 - Performance tests for ByteArray: Patch 2: more microbenchmarks (r=fklockii)

http://hg.mozilla.org/tamarin-redux/rev/fba3ca915295

Comment 16

[Tamarin Bot]

changeset: 6653:5968a1240330
user:      Lars T Hansen <[e-mail]>
summary:   For 687046 - Performance tests for ByteArray: Canary for ByteArray: vector->bytevector, bytevector->vector (r=fklockii)

http://hg.mozilla.org/tamarin-redux/rev/5968a1240330

Comment 17

[Lars T Hansen]

Keeping the bug open because there's more work to do.

The benchmark set is missing a few things, notably these:

- write* that extend the buffer
- readUTF / readUTFBytes / writeUTF / writeUTFBytes with non-7-bit data

Also, for a given set of tests bytearray-{read,write}-X-*.as the workload is intended to be constant, but that does not hold at this time for readUTF / readUTFBytes.  I may or may not wish to fix that, by and by.

Comment 18

[Lars T Hansen]

... but those additional items can wait until Cyril.

Comment 19

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.

Comment 20

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.

Comment 21

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.

Comment 22

[Sylvestre Ledru [:Sylvestre]]

Tamarin is a dead project now. Mass WONTFIX.

Comment 23

[Sylvestre Ledru [:Sylvestre]]

Tamarin isn't maintained anymore. WONTFIX remaining bugs.