Closed
Bug 687046
Opened 13 years ago
Closed 6 years ago
Performance tests for ByteArray
Categories
(Tamarin Graveyard :: Library, defect, P3)
Tamarin Graveyard
Library
Tracking
(Not tracked)
RESOLVED
WONTFIX
Q2 12 - Cyril
People
(Reporter: lhansen, Unassigned)
References
Details
Attachments
(4 files)
126.33 KB,
patch
|
pnkfelix
:
review+
|
Details | Diff | Splinter Review |
17.38 KB,
patch
|
pnkfelix
:
review+
|
Details | Diff | Splinter Review |
36.74 KB,
patch
|
pnkfelix
:
review+
|
Details | Diff | Splinter Review |
16.63 KB,
patch
|
pnkfelix
:
review+
|
Details | Diff | Splinter Review |
We need microbenchmarks and canaries for the ByteArray performance work.
Reporter | ||
Comment 1•13 years ago
|
||
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 2•13 years ago
|
||
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•13 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•13 years ago
|
||
First batch of benchmarks landed in Brannan, so targeting.
Priority: -- → P3
Target Milestone: --- → Q1 12 - Brannan
Reporter | ||
Comment 5•13 years ago
|
||
Keeping this open: more benchmarks are coming.
Reporter | ||
Comment 6•13 years ago
|
||
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•13 years ago
|
||
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•13 years ago
|
||
Inability of test/performance/runtests.py to handle .abs files filed as bug #692718.
Reporter | ||
Comment 9•13 years ago
|
||
Adds ByteArray microbenchmarks: write "position" (important for random access use case), writeUTF, writeUTFBytes.
Attachment #565458 -
Flags: review?(fklockii)
Comment 10•13 years ago
|
||
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 11•13 years ago
|
||
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 12•13 years ago
|
||
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•13 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•13 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•13 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•13 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•13 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•13 years ago
|
||
... but those additional items can wait until Cyril.
Target Milestone: Q1 12 - Brannan → Q2 12 - Cyril
Reporter | ||
Updated•12 years ago
|
Assignee: lhansen → nobody
Comment 20•6 years ago
|
||
No assignee, updating the status.
Comment 21•6 years ago
|
||
No assignee, updating the status.
Comment 22•6 years ago
|
||
Tamarin is a dead project now. Mass WONTFIX.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
Comment 23•6 years ago
|
||
Tamarin isn't maintained anymore. WONTFIX remaining bugs.
You need to log in
before you can comment on or make changes to this bug.
Description
•