698721 - Use getNanosecondTimer() instead of new Date() in all performance tests code paths

Status: RESOLVED WONTFIX

(Tamarin Graveyard :: Tools, defect, P3)

Description

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

Many of the performance tests use the pattern
  var a = new Date();
  runTest();
  var b = new Date();
  var elapsedTime = b - a;

It would be much easier to switch our performance tests to use more accurate performance counters (see Bug 625030) if the tests were using getTimer() rather than new Date().

One example of a place where the 'new Date()' pattern was introduced was in changeset 1521:fd97f5127083 (hg.mozilla.org/tamarin-redux/rev/1521) from Bug 479977.  One can see in Bug 479977, comment 2 and Bug 479977, comment 4, there was some discussion even then about switching to using getTimer() across the board.  (Or more generally, deciding on one or the other and sticking to it.)

I'm pretty sure we'll have an easier time coaxing getTimer() to provide us with the most precise results.  Hopefully they'll even be accurate.  This in turn will save engineers time from investigating supposed "regressions" that are in fact just instances of particularly noisy benchmarks (e.g. sunspider-0.9.1/typed/bitops-bitwise-and)

Comment 1

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

(I could understnad wanting to keep the code that originated and remains pure Javascript in a form that is still compatible; but it also seems really easy to throw in a 'function getTimer() { return Date.getTime(); }' in the driver for the file in such cases.)

Comment 2

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

A potentially useful "one-liner" that I'll stash away here in case this bug gets lost in immediate shuffle:

  % for f in $(find . -exec grep -l 'new Date' {} \; | xargs) ; do echo $f ; mkdir -p /tmp/foo/$(dirname $f) ; cat $f | perl -p -i -e 's/var start(Time)?(:uint)? = new Date\(\)(.getTime\(\))?;/var start$1:uint = getTimer();/; $startSuffix = $1; s/var elapsed(Time)?(:uint)? = new Date\(\)(.getTime\(\))? - start(Time)?;/var elapsed$1:uint = getTimer() - start$startSuffix;/; s/var time(:uint)? = new Date\(\)(.getTime\(\))? - start(Time)?;/var time:uint = getTimer() - start$startSuffix;/' > /tmp/foo/$f ; cat /tmp/foo/$f > $f ; done

It does not catch all the cases; the other cases will have to be addressed by hand.

(Also the code it produces can in many cases be refactored into something short [e.g. folding instances of 'if (CONFIG::desktop) { ... } else { ... }' into just an inner (CONFIG::desktop ? 0 : 3)], but I think I'm better off making this rewrite as direct and dumb as possible and save the refactoring for later.)

Comment 3

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

(well there are some pretty direct extensions to the above regexp that should increase its coverage.  I'll stop posting it as comments here; if a larger form has significantly better coverage, I'll post it as a proper script attachment.)

Comment 4

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

Attachment: script P v1: convert uses of new Date to getTimer()

This script seems to cover every case except two: misc/boids.as and mmgc/gcbench.as.

That is, after running it, I manually inspected of every occurence of 'new Date' in .as files in test/performance/ other than the above two (there's 105 of them, but they fall into easily recognizable categories), and they're all unrelated to the benchmark timing.

Comment 5

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

(In reply to Felix S Klock II from comment #4)
> Created attachment 571024 [details]
> script P v1: convert uses of new Date to getTimer()

FYI this is approximately how I've been running the script:

  % for f in $(find . -exec grep -l 'new Date' {} \; | xargs) ; do \
      echo $f ; mkdir -p /tmp/foo/$(dirname $f) ;                  \
      cat $f | perl conv.perl > /tmp/foo/$f ; cat /tmp/foo/$f > $f ; done

Comment 6

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

Attachment: patch Q v1: fix boids and gcbench separately first

Comment 7

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

Attachment: patch R v1: (generated) replace other timing uses of "new Date"

Comment 8

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

Attachment: .numbers spreadsheet N v1: sunspider + language/e4x, 99 iters

Comment 9

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

Attachment: Npdf: PDF rendering of attachment 571294

Comment 10

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

Attachment: .numbers spreadsheet N v2: sunspider + language/e4x, 99 iters

(added table showing how much the noise/signal is reduced by the getTimer change on its own, and by then by the hires getTimer change on top of that.)

Comment 11

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

Attachment: .numbers spreadsheet N v3: sunspider + language/e4x, 99 iters

(added geometric means for averaging SNR because I remembered arithmetic mean may not be a valid way to combine those quantities and I don't want to take the time at the moment to work out which one is right.  I'm handling 0 SNR values in the data by treating them as 0.00001, since geometric mean requires positive input values.)

Comment 12

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

Attachment: Npdf: PDF rendering of attachment 571304

Updated pdf rendering of spreadsheet.

My main takeaways from this data:

* The actual problem that originally motivated me to look into this again was the noisiness (or at least high degree of variation) of sunspider/as3/bitops-bitwise-and.  Unfortuantely, switching to a hires timer does not reduce the variation in that case at all.  :(

* Nor does it reduce the variation in some other outlier cases of very high variation in the results, namely: as3/bitops-3bit-bits-in-byte (1/SNR=0.11), as3vector/bitops-nsieve-bits (1/SNR=0.12), as3vector/access-nbody (1/SNR=0.14), and e4x/simpleStringConcatenation (1/SNR=0.35 (!)).

* However, switching to using hi-res getTimer *does* reduce the variation quite significantly in a large number of cases (all the way to 0 variation in a few).

* The geometric mean noise/signal went from 0.026 to 0.012.  The arithmetic-mean noise/signal went from 0.045 to 0.034.  (See comment 11.)

* All of the cases where the variation is increased rather than reduced are cases where the amount of variation was already small, and remained small.

So I'd probably argue that putting these changes in is a net win, and I plan to do future PACMAN performance evaluation on top of a patch queue with these changes applied.

Comment 13

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

Comment on attachment 571291 [details] [diff] [review]
patch Q v1: fix boids and gcbench separately first

(this is not a high priority review; i.e. it can wait until after float-landing week.  I just figured I should get it into the pipeline.)

Comment 14

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

Comment on attachment 571292 [details] [diff] [review]
patch R v1: (generated) replace other timing uses of "new Date"

(this is not a high priority review; i.e. it can wait until after float-landing week.  I just figured I should get it into the pipeline.)

Comment 15

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

Comment on attachment 571304 [details]
.numbers spreadsheet N v3: sunspider + language/e4x, 99 iters

(In case its not clear, the experiments here were done just using the #define PERFORMANCE_GETTIMER; see Bug 625030, comment 11.  Bug 625030 may get updated later with even-more precise timing mechanisms.)

Comment 16

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

Comment on attachment 571292 [details] [diff] [review]
patch R v1: (generated) replace other timing uses of "new Date"

retracting review request.  The code runs fine in our harness, but the automated rewrite includes some bogus stuff like obfuscatedly calling getTimer().getTime().

I'll fix before rerequesting review.

Comment 17

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

Comment on attachment 571291 [details] [diff] [review]
patch Q v1: fix boids and gcbench separately first

(might as well hold off on this one too)

Comment 18

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

(In reply to Felix S Klock II from comment #12)
> Unfortuantely, switching to a hires timer does not reduce the variation in
> [sunspider/as3/bitops-bitwise-and].  :(
> [...]
> * Nor does it reduce the variation in some other outlier cases of very high
> variation in the results, namely: as3/bitops-3bit-bits-in-byte (1/SNR=0.11),
> as3vector/bitops-nsieve-bits (1/SNR=0.12), as3vector/access-nbody
> (1/SNR=0.14), and e4x/simpleStringConcatenation (1/SNR=0.35 (!)).

(I am seeing evidence now that the easiest way to resolve this may be to add a submillisecond timer (i.e. one with nano- or micro-second resolution), as was originally suggested in Bug 625030.  The data need not be reported using such a fine resolution; we benefit just from using the finer grain during for the internal measurements.)

Comment 19

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

Attachment: .numbers spreadsheet N v4: sunspider + language/e4x, 99 iters

Added runs with getNanosecondTimer; *this* is more like what I was expecting with a switch to a high resolution timer.  (And this makee sense; its not enough to just enable access to the other timer; one should also change to the more precise measure; otherwise you are going to get similar sampling distortion.)

Comment 20

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

Attachment: Npdf: PDF summarizing attachment 571332

This summary presents the much more significant reduction in variation when using getNanosecondsTimer instead of getTimer for the benchmarking.

Like I said in previous comment: this is more like what I was hoping to see with the switch to a high resolution timer.

sunspider/as3/bitops-bitwise-and, when using getNanosecondTimer, has 1/SNR=0.0027 (as opposed to the former 0.28).  That's huge!

as3/bitops-3bit-bits-in-byte:  0.0023 (before was 0.11)
as3vector/bitops-nsieve-bits:  0.0029 (before was 0.12)
as3vector/access-nbody:        0.0050 (before was 0.14)
e4x/simpleStringConcatenation: 0.0041 (before was 0.35)

The geometric mean 1/SNR is 0.0032; the arithmetic mean 1/SNR is 0.0049.

There are some cases that still have 1/SNR >= 0.01: {date-format-tofte, as3/date-format-tofte, s3d-morph, as3/s3d-morph, e4x/addingToXMLList}.  But that's still way way better than what I was seeing before.

There is a drawback: the reported elapsed times have increased from the variant that uses getTimer, consistently by 10% or so.  I don't have an explanation for that yet (one hypothesis is that my replacement script has replaced some calls to getTimer with getNanosecondsTimer in an inner loop that is not using types to ensure the doubles do not get boxed.  I might believe that for some outlier cases, but this is occurring it seems across all of sunspider and language/e4x).

Comment 21

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

Attachment: patch S v1: replace uses of getTimer with getNanosecondTimer

(patch S is meant to be applied atop patch Q+R, since it was sort of simpler to just start with a bunch of getTimer calls to replace.  I'll post the perl script I used for the majority of this patch shortly.)

This patch is what I used to benchmark the shell to generate the dat in the 'nanotime' column in spreadsheet Nv4 (attachment 571332 [details]).

Comment 22

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

(In reply to Felix S Klock II from comment #20)
> There is a drawback: the reported elapsed times have increased from the
> variant that uses getTimer, consistently by 10% or so.  I don't have an
> explanation for that yet (one hypothesis is that my replacement script has
> replaced some calls to getTimer with getNanosecondsTimer in an inner loop
> that is not using types to ensure the doubles do not get boxed.  I might
> believe that for some outlier cases, but this is occurring it seems across
> all of sunspider and language/e4x).

The slowdown appears to be introduced merely by adding getNanosecondTimer to the shell code (via Bug 625030, attachment 571328 [details] [diff] [review]), *without* even having to change the calling code at all.

That is, after enabling hires timers but before adding getNanosecondTimer, I get a time of 8ms for sunspider/as3/bitops-bits-in-byte, but after adding getNanosecondTimer to the shell code, the benchmark immediately reports 9ms.  (In both cases there is zero variation in the reported value in both cases over 99 iterations.)

Comment 23

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

And if I revise the getNanosecondTimer patch (patch N on Bug 625030, attachment 571328 [details] [diff] [review]) so that it *just* adds a method to the System class but does not define getNanosecondTimer at the top level of the package in shell_toplevel.as, then the results (again for just calling getTimer()) are 8ms for sunspider/as3/bitops-bits-in-byte.

I suspect there are overheads to making toplevel calls that I am either unaware of or have forgotten.  Nonetheless I am quite suprised that adding a new toplevel function would cause such a significant change in benchmark results.

Q1: does this all loop back to the planned find-def optimization?  Or is that unrelated (since that's about speeding up the time it takes to look up a class, e.g. Math in 'Math.abs(x)', but invocations of toplevel functions do not have a class attached to them)?

Q2: would i further reduce noise by making direct invocations of System.getTimer and analogously System.getNanosecondsTimer rather than the toplevel function?  (Seems quite possible at this point.)

Comment 24

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

Attachment: script N v1: convert uses of getTimer() to getNanosecondsTimer

(This is the perl script I promised in comment 21.  My subsequent investigation described in comment 22 and 23 has indicated that I'll probably not use this script as written, since adding getNanosecondTimer to the toplevel of the shell introduces slow-down.  So I am posting it mostly for completeness and because it can be easily adapted to produce output that calls to System.getNanosecondTimer() instead of just getNanosecondTimer().)

Comment 25

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

changeset:   6696:b68cf8738b41
user:        Felix S Klock II <[e-mail]>
date:        Thu Nov 03 00:50:41 2011 +0100
summary:     Bug 698721: drive-by run of fixtabs on perf tests (r=fklockii).

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

Comment 26

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

(In reply to Felix S Klock II from comment #25)
> changeset:   6696:b68cf8738b41
> user:        Felix S Klock II <[e-mail]>
> date:        Thu Nov 03 00:50:41 2011 +0100
> summary:     Bug 698721: drive-by run of fixtabs on perf tests (r=fklockii).
> 
> http://hg.mozilla.org/tamarin-redux/rev/b68cf8738b41

*Bizarre* : running fixtabs has broken jsbench/Euler.as.

willfix.

Comment 27

[Tamarin Bot]

changeset: 6705:11663e0ba6f0
user:      Felix S Klock II <[e-mail]>
summary:   Bug 698721: fix Euler perf tests by reintroducing tabs in literal strings that had been boiled away by fixtabs (r=fklockii).

http://hg.mozilla.org/tamarin-redux/rev/11663e0ba6f0

Comment 28

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

Attachment: script P v2: convert new Date/getTimer to getNanosecondTimer

Revised script; it now converts a wide variety of time-generated expressions to use getNanosecondTimer instead, and makes all replaced variable bindings use a :Number type declaration so that we avoid boxing the double returned by getNanosecondTimer().
 
The script works hard to preserve spacing and variable names.

It does *not* automatically produce working output; the modified files need to be subsequently cleaned up after applying the script, because:

- Many need an import of avmplus.System so that the System.getNanosecondTimer call succeeds.

- A few (asmicro, v8*) need to have some time calculations adjusted to use nanosecond units as input to time conversions/comparisons rather than millisecond units.

(I decided its easier to do the latter as manual rewrites rather than trying to generalize the script to those cases, because the first bullet is actually surprising hard to automate properly, and the second bullet is such few cases that it does not motivate automation.)

Comment 29

[Garnet Chaney]

Fresh in my mind still is the time I spent a few months ago looking into a test case where 2/3rd of the time, the GetTimer() measurement of some code was rounded down to 0. In a different shell it was maybe 3/4th or 4/5ths of the time rounded to zero. I figured with 5000 trials, maybe the ratio of 0 to 1 was still a somewhat accurate characterization of the time, but only in relative terms between different versions of the shell. But eventually it seemed clear to me that a better solution would be to redesign the test to time larger chunks of execution.

With 5000 trials, 4/5ths of the measurements being rounded to zero, everything still completed in a second or so, a large enough period of time that a resolution of nanoseconds would be false precision. And the overall time to complete the tests on that issue being less than 10 or 20 seconds, it didn't seem like a huge burden that would encourage me to use less iterations to get the same result in just 1 or 2 seconds.

But I also concluded there is enough jitter in background things happening on a typical machine that performance testing of small intervals is still difficult to do in a reproducible way. It's also easy to forget about things like JIT effects on timing. And with the move to infrastructure as a service, there are all kinds of traps waiting if someone tries to use virtual machines for performance testing.

I wonder if your change will encourage test engineers to think there is more validity in timing smaller chunks of code than there really is. Your comment about enabling smaller numbers of iterations to be used seems to indicate encouraging this tendency.

Perhaps in your documentation for the new function, some mention needs to be made of the trade off between measuring really small chunks of code and overhead, versus timing larger chunks of code.

Comment 30

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

(In reply to Garnet Chaney from comment #29)
> But eventually it
> seemed clear to me that a better solution would be to redesign the test to
> time larger chunks of execution.

Yep, and when this is an option, I would prefer to do things this way.

But in the cases that have been vexing me (e.g. sunspider/*; see comment 20), revising the test is an option, because I believe we want the results to be comparable.

(I could make a new version of such noisy tests that run for longer periods of time, and avoid running the noisy old versions that I think are unreliable.  But I prefer the option of using getNanosecondTimer(), at least for now.)

Comment 31

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

Attachment: patch Q v2: generated (via script P) fixes for most perf tests

Comment 32

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

Attachment: patch R: fix asmicro driver.as for ns measure

Comment 33

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

Attachment: patch S: fix misc driver.as for ns measure

Comment 34

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

Attachment: patch T: add import avmplus.System to files with getNanosecondTimer

Comment 35

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

Attachment: patch U: fix v8 base.as for ns measure

Comment 36

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

Attachment: patch V: switch jsmicro to an actionscript driver.as with ns measure

Patches Q through V are a checkpoint of my current queue of patches.

You need to apply at least patches Q through U to get things working.  Patch V is optional gravy on top of it.

Discussion with the team has led me to conclude that in the short term, it is not safe for me to blindly revise the .as code in js/ directories: sunspider-0.9.1/js/ and v8.5/js/.  I will need to change these patches (starting with Q) to not convert those files to use getNanosecondTimer.  That work has not been done yet; I'll wait until I've done that to start the review process.

(Long term I'd argue that if some code is meant to be contrained to Javascript syntax/features, then it should be in a foo.js file that is then included by foo.as for the test infrastruture.  But this is really a policy issue that is not my decision and not something terrible important to me.)

Comment 37

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

Attachment: .numbers spreadsheet O v1: all benchmarks, 20 iters

Comment 38

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

Attachment: Opdf: PDF summarizing attachment 572459

Comment 39

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

Executive summary of the data in O (attachment 572459 [details], attachment 572460 [details]):

The uploaded spreadsheet is sorted with the noisiest tests first (noisiest *after* the patch landed).  The noisiest cases tended to be noisy before the patch landed as well.

The average noise reduction across all the tests, using geometric mean, is 0.61.  So its not like the noise reduction is as striking across the board as one might think from looking at the isolated data from N (attachment 571332 [details], attachment 571339 [details]).  But as I said before, in the cases where it helps, it helps a *lot*:

                                            std dev	std dev
                                            before	after
                                            -------	-------
jsmicro/number-toString-2                   7.859	0.034
asmicro/parseFloat-1                        3.281	0.031
jsmicro/oop-1.as                            0.450	0.006
sunspider-0.9.1/typed/bitops-bitwise-and.as 0.510	0.014
jsmicro/number-toString-1.as                0.584	0.016
sunspider-0.9.1/typed/math-partial-sums.as  0.510	0.016
sunspider/as3/access-nbody.as               0.410	0.013
sunspider/as3/bitops-bitwise-and.as         0.410	0.016
language/e4x/simpleStringConcatenation.as   0.489	0.018

There are some cases where the signal-to-noise (SNR) ratio went down (which is presented in the spreadsheet as a rise in the inverse 1/SNR).  But in nearly all such cases the amount of noise is already low enough that the reduction in SNR itself could very well be noise (or artifacts of the bucketing inherent in using a coarse-grain timer).

Anyway, I plan on trying to land this this week.  (My next step is to revise the attached patches so that they do not touch the files in directories named js/, and then after a rebase I'll start the review process.)

Comment 40

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

(In reply to Felix S Klock II from comment #39)
> There are some cases where the signal-to-noise (SNR) ratio went down (which
> is presented in the spreadsheet as a rise in the inverse 1/SNR).  But in
> nearly all such cases the amount of noise is already low enough that the
> reduction in SNR itself could very well be noise (or artifacts of the
> bucketing inherent in using a coarse-grain timer).

FYI the main exception to this claim is jsmicro/globalvar-write-1.as (since the noise is not low at all in this case);

  std dev  std dev    mean    mean      100/snr  100/snr    noise
  before   after      before  after     before   after      ratio
  -------  -------    ------  ------    -------  -------    -----
  44.838   73.936     504.13  477.26    8.894	 15.492     1.742

(Of course, this is one of the notoriously noisy tests that myself I put in 1.5 years ago and have seen regretted not deleting somewhere along the way since then.)

But most of the other cases with a noise ratio significantly above 1.0 are cases where the noise level was low.  (The only other candidates for investigation where this rule does not apply are asmicro/alloc-8, asmicro/restarg-2, and sunspider/s3d-morph.)

Comment 41

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

(In reply to Felix S Klock II from comment #20)
> There is a drawback: the reported elapsed times have increased from the
> variant that uses getTimer, consistently by 10% or so.  I don't have an
> explanation for that yet (one hypothesis is that my replacement script has
> replaced some calls to getTimer with getNanosecondsTimer in an inner loop
> that is not using types to ensure the doubles do not get boxed.  I might
> believe that for some outlier cases, but this is occurring it seems across
> all of sunspider and language/e4x).

(In reply to Felix S Klock II from comment #23)
> And if I revise the getNanosecondTimer patch (patch N on Bug 625030,
> attachment 571328 [details] [diff] [review] [diff] [details] [review]) so that it *just* adds a method
> to the System class but does not define getNanosecondTimer at the top level
> of the package in shell_toplevel.as, then the results (again for just
> calling getTimer()) are 8ms for sunspider/as3/bitops-bits-in-byte.

The above two comments were analyzing the data from running just the sunspider and language/e4x benchmarks.  To save people the effort of extracting the data from the posted spreadsheet, here are the regressions (legend: 'orig' is the baseline runs, 'nano' is the run after adding a new toplevel getNanosecondsTimer (and calling that instead, which based on comment 23 I am assuming is not the actual cause of the regression, but rather the addition of the function itself), and 'regress' = 'nano/orig'.  So in the regress column, >1 is bad, and >1.1 means a more than 10% regression.

testname	                  	orig 	nano	regress
----------------------------------	-----	-----	-------
            as3/access-fannkuch.as	37.57	42.19	1.12
                     crypto-md5.as	16.36	18.32	1.14
   as3/bitops-3bit-bits-in-byte.as	4.56	5.23	1.22
      as3/string-validate-input.as	30.21	34.03	1.12
                   as3/s3d-cube.as	17.34	19.53	1.14
        as3/access-binary-trees.as	8.32	9.37	1.16
       e4x/appendChildAndString.as	43.77	48.62	1.12
            access-binary-trees.as	31.26	35.11	1.13
                  as3/s3d-morph.as	23.99	28.66	1.20
                     crypto-aes.as	32.96	36.77	1.11
              as3/access-nsieve.as	12.53	14.07	1.15
                   string-fasta.as	71.74	88.02	1.22
as3vector/string-validate-input.as	31.90	34.98	1.09
          string-validate-input.as	39.92	45.40	1.13
          as3/math-partial-sums.as	71.49	80.64	1.13
      as3vector/access-fannkuch.as	8.85	9.90	1.10
         as3/bitops-nsieve-bits.as	23.11	26.06	1.13
                  access-nsieve.as	19.86	22.32	1.12
         as3/bitops-bitwise-and.as	1.65	1.83	1.09
                as3/crypto-sha1.as	14.88	16.88	1.12
              date-format-tofte.as	151.79	170.45	1.12
       bitops-3bit-bits-in-byte.as	8.62	9.71	1.16
         as3/string-unpack-code.as	150.82	167.85	1.12
          as3/date-format-tofte.as	145.55	162.17	1.11
         as3vector/string-fasta.as	18.65	21.03	1.10
                access-fannkuch.as	59.59	67.32	1.13
               as3/s3d-raytrace.as	28.80	31.99	1.10
               as3/access-nbody.as	4.76	5.38	1.08
        as3vector/access-nsieve.as	9.31	10.48	1.16
          controlflow-recursive.as	13.86	15.54	1.11
                as3/math-cordic.as	12.64	14.43	1.11
         as3/math-spectral-norm.as	4.26	4.79	1.20
            bitops-bits-in-byte.as	23.90	26.94	1.12
                   s3d-raytrace.as	68.48	75.25	1.10
      as3/controlflow-recursive.as	3.24	3.62	1.16
   as3vector/bitops-nsieve-bits.as	3.57	3.97	1.25
            as3vector/s3d-morph.as	11.11	12.49	1.13
e4x/concatenatingStringsFromE4X.as	6.35	7.27	1.15
                 as3/crypto-aes.as	27.02	29.96	1.11
              math-partial-sums.as	213.72	244.85	1.16
             math-spectral-norm.as	22.80	25.74	1.15
   as3vector/math-spectral-norm.as	10.63	12.26	1.11
                       s3d-cube.as	65.61	72.26	1.11
        as3/bitops-bits-in-byte.as	7.43	8.98	1.28
             bitops-nsieve-bits.as	33.02	37.29	1.13
         as3vector/access-nbody.as	3.53	3.97	1.11
                      s3d-morph.as	31.91	37.40	1.16
                 as3/crypto-md5.as	21.01	26.87	1.26
                   access-nbody.as	69.94	78.79	1.13
                    crypto-sha1.as	15.93	17.50	1.09
     e4x/usingAppendChildAndE4X.as	45.68	51.29	1.13
                    math-cordic.as	47.17	52.77	1.11
          as3vector/math-cordic.as	9.85	11.13	1.11
             as3vector/s3d-cube.as	19.39	21.73	1.09
             string-unpack-code.as	150.34	167.61	1.12
  e4x/simpleStringConcatenation.as	1.34	1.55	1.21
               as3/string-fasta.as	35.90	40.33	1.11
            e4x/addingToXMLList.as	14.96	16.58	1.10
            bitops-bitwise-and.as	228.80	253.34	1.11

Comment 42

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

(In reply to Felix S Klock II from comment #41)
> (In reply to Felix S Klock II from comment #20)
> > There is a drawback: the reported elapsed times have increased from the
> > variant that uses getTimer, consistently by 10% or so.  I don't have an
> > explanation for that yet (one hypothesis is that my replacement script has
> > replaced some calls to getTimer with getNanosecondsTimer in an inner loop
> > that is not using types to ensure the doubles do not get boxed.  I might
> > believe that for some outlier cases, but this is occurring it seems across
> > all of sunspider and language/e4x).
> 
> (In reply to Felix S Klock II from comment #23)
> > And if I revise the getNanosecondTimer patch (patch N on Bug 625030,
> > attachment 571328 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review]) so that it *just* adds a method
> > to the System class but does not define getNanosecondTimer at the top level
> > of the package in shell_toplevel.as, then the results (again for just
> > calling getTimer()) are 8ms for sunspider/as3/bitops-bits-in-byte.

Well, I don't know what the cause of the 10% regression reported above was, but I cannot reproduce it now.

So I'm not going to worry about the hypothetical impact of adding new toplevel function definitions until I see evidence of a regression surface again.

Comment 43

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

Arguably there is probably stuff here that the VM team and/or Performance team could salvage.  But it seems simplest to admit that it just isn't going to get fixed.