594014 - Sundry formatting problems in the output of runtests.py

Status: VERIFIED FIXED

(Tamarin Graveyard :: Tools, defect)

Description

[Lars T Hansen]

Consider this:

lhansen-MacPro:performance lhansen$ ASC=~/lib/asc.jar AVM=../../platform/mac/avmshell/build/Release/avm GLOBALABC=../../core/builtin.abc SHELLABC=../../shell/shell_toplevel.abc ./runtests.py --iterations=5 jsbench
Tamarin tests started: 2010-09-07 15:49:35.027571
Executing 21 test(s)
avm: ../../platform/mac/avmshell/build/Release/avm  version: cyclone
iterations: 5


test                                                 avm    avg 95%_conf

Metric: time 
Dir: jsbench/
  Crypt                                               3404   3408  0.1% 
  Euler                                               6256  6302.4  0.5% 
  FFT                                                 6445  6521.2  1.1% 
  HeapSort                                            2956  2963.2  0.3% 
  LUFact                                              6972   7025  0.7% 
  Moldyn                                              7840  7848.8  0.1% 
  RayTracer                                           6442  6462.2  0.2% 
  SOR                                                27964  28134  0.8% 
  Series                                              7249  7288.6  0.5% 
  SparseMatmult                                      11173  11197.4  0.2% 

Here are two problems with the output:

(a) what does column 2 represent?  The heading 'avm' is not useful.

(b) When the average is an integer the spacing is off.  I expect always printing one decimal even if it is ".0" is probably the best fix.

Comment 1

[James Sudduth]

Attachment: Fix perfromance runntests.py formatting problems.

Comment 2

[Dan Schaffer]

Comment on attachment 537254 [details] [diff] [review]
Fix perfromance runntests.py formatting problems.

the patch looks good.  When testing I found a problem with this case:
$ ./runtests.py --iterations=2 v8.5/optimized/

Dir: v8.5/optimized/
  crypto                                              3308 3306.0  0.5% 

Keyboard Interrupt
Traceback (most recent call last):
  File "./runtests.py", line 1043, in <module>
    runtest = PerformanceRuntest()
  File "./runtests.py", line 139, in __init__
    RuntestBase.__init__(self)
  File "../util/runtestBase.py", line 218, in __init__
    self.run()
  File "./runtests.py", line 164, in run
    self.runTests(self.tests)
  File "./runtests.py", line 453, in runTests
    o = self.runTest((t, testnum))
  File "./runtests.py", line 773, in runTest
    self.printTestResults(testName)
  File "./runtests.py", line 863, in printTestResults
    [self.formatResult(x, metric=metric) for x in self.testData[testName][metric]['results1']] if self.raw else ''
TypeError: float argument required, not str

Comment 3

[James Sudduth]

Attachment: Fix float argument problem, achieve better formatting.

This fixes the float argument problem by removing the integer handling code from the formatResult() function, forcing it to always return a string formatted as a float. I left in the old formatResult() function for now until we see how this all works in production; I'll remove it later if all looks well. 
The patch also spaces the header titles a bit better, changes 'avm' to 'best' and forces all number columns to right-justify. Occasionally header title/number column vertical alignment will be slightly off but it's better than before IMO. I've tested this with jsbench, mmgc, sunspider, v8, asmicro and misc.

Comment 4

[Tamarin Bot]

changeset: 6381:7d31ad6f0fa5
user:      James Sudduth <[e-mail]>
summary:   Bug 594014 - Sundry formatting problems in the output of runtests.py (r=dschaffe)

http://hg.mozilla.org/tamarin-redux/rev/7d31ad6f0fa5

Comment 5

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

Before your patch, doing a single iteration with two shells would produce this output:

iterations: 1

test                          avm    avm2   %diff

Metric: iterations/second 
Dir: asmicro/
  alloc-1                     49.2    41.5  -15.7  -15.7


Now it produces this:

iterations: 1

test                         best    avm2   %diff

Metric: iterations/second 
Dir: asmicro/
  alloc-1                     49.2    41.5  -15.6  -15.6


It is not terrible that the fourth column is unlabelled, since it always has the same contents as the third column.  (And that's the way it was before, AFAICT).

But I'm not a fan of the fact that "avm" has been replaced with "best"; with iterations=1 and using avm2.

I can understand the problem described in the bug description, and your solution makes sense in the case where avm2 is unsupplied and iterations > 1.  But it has broken the output when avm2 is supplied and iterations == 1.  (Arguably it is bad output for iterations == 1 regardless of whether avm2 is supplied; it is meaningless to write "best", since its only one iteration, there is no "best".  But that's a nitpick.)

Should we consider making the output match the two line format it uses with iterations>1 and an avm2 supplied?  For example, here's what it does for iterations==2 and with avm2 supplied:

iterations: 2

                                        avm            avm2
test                           best     avg    best     avg   %dBst   %dAvg
Metric: iterations/second 
Dir: asmicro/
  alloc-1                      49.0    49.0    41.2    41.1   -15.9   -16.1 --

Comment 6

[James Sudduth]

Attachment: Change column name, fix some column formatting problems.

I've come up with "result" to address the problem with "best"; admittedly not inspired, if anyone can think of a better term please let me know.
 
Changed the header when one avm is run for multiple iterations to be similar to  that when two avms are run. 

Also fixed a few column alignment problems that showed up in testing.

Comment 7

[Brent Baker]

Comment on attachment 545951 [details] [diff] [review]
Change column name, fix some column formatting problems.

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

Comment 8

[Tamarin Bot]

changeset: 6467:3ce9cae73af8
user:      James Sudduth <[e-mail]>
summary:   Bug 594014 - Sundry formatting problems in the output of runtests.py (r=brbaker)

http://hg.mozilla.org/tamarin-redux/rev/3ce9cae73af8