Performance tests for ByteArray

RESOLVED WONTFIX

Status

P3
normal
RESOLVED WONTFIX
7 years ago
7 days ago

People

(Reporter: lhansen, Unassigned)

Tracking

unspecified
Q2 12 - Cyril

Details

Attachments

(4 attachments)

(Reporter)

Description

7 years ago
We need microbenchmarks and canaries for the ByteArray performance work.
(Reporter)

Comment 1

7 years ago
Created attachment 560912 [details] [diff] [review]
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*.
Attachment #560912 - Flags: review?(fklockii)
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.
Attachment #560912 - Flags: review?(fklockii) → review+

Comment 3

7 years ago
changeset: 6592:a4f648fa8281
user:      Lars T Hansen <lhansen@adobe.com>
summary:   For 687046 - Performance tests for ByteArray: first batch (r=fklockii)

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

Comment 4

7 years ago
First batch of benchmarks landed in Brannan, so targeting.
Priority: -- → P3
Target Milestone: --- → Q1 12 - Brannan
(Reporter)

Comment 5

7 years ago
Keeping this open: more benchmarks are coming.
(Reporter)

Comment 6

7 years ago
Created attachment 565168 [details] [diff] [review]
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.
Attachment #565168 - Flags: review?(fklockii)
(Reporter)

Comment 7

7 years ago
Created attachment 565315 [details] [diff] [review]
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.)
Attachment #565315 - Flags: review?(fklockii)
(Reporter)

Comment 8

7 years ago
Inability of test/performance/runtests.py to handle .abs files filed as bug #692718.
(Reporter)

Comment 9

7 years ago
Created attachment 565458 [details] [diff] [review]
Patch 2: more microbenchmarks

Adds ByteArray microbenchmarks: write "position" (important for random access use case), writeUTF, writeUTFBytes.
Attachment #565458 - Flags: review?(fklockii)
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.
Attachment #565168 - Flags: review?(fklockii) → review+
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.
Attachment #565315 - Flags: review?(fklockii) → review+
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.)
Attachment #565458 - Flags: review?(fklockii) → review+
(Reporter)

Comment 13

7 years ago
(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

7 years ago
changeset: 6651:00990d795cd1
user:      Lars T Hansen <lhansen@adobe.com>
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

7 years ago
changeset: 6652:fba3ca915295
user:      Lars T Hansen <lhansen@adobe.com>
summary:   For 687046 - Performance tests for ByteArray: Patch 2: more microbenchmarks (r=fklockii)

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

Comment 16

7 years ago
changeset: 6653:5968a1240330
user:      Lars T Hansen <lhansen@adobe.com>
summary:   For 687046 - Performance tests for ByteArray: Canary for ByteArray: vector->bytevector, bytevector->vector (r=fklockii)

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

Comment 17

7 years ago
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.
(Reporter)

Comment 18

7 years ago
... but those additional items can wait until Cyril.
Target Milestone: Q1 12 - Brannan → Q2 12 - Cyril
(Reporter)

Updated

7 years ago
Assignee: lhansen → nobody
No assignee, updating the status.
Status: ASSIGNED → NEW
No assignee, updating the status.
No assignee, updating the status.
Tamarin is a dead project now. Mass WONTFIX.
Status: NEW → RESOLVED
Last Resolved: 7 days ago
Resolution: --- → WONTFIX
Tamarin isn't maintained anymore. WONTFIX remaining bugs.
You need to log in before you can comment on or make changes to this bug.