Changed the RegExp benchmark to exercise the regexp engine on different ...

Changed the RegExp benchmark to exercise the regexp engine on different 
inputs by scrambling the input strings.

Committed: http://code.google.com/p/v8/source/detail?r=5493

[Kasper Lund, 2010-09-19 12:45:17]

LGTM if you address these comments:

http://codereview.chromium.org/3435012/diff/1/5
File benchmarks/README.txt (right):

http://codereview.chromium.org/3435012/diff/1/5#newcode74
benchmarks/README.txt:74: benchmark to exercise the regexp engine on different
input strings.
regexp => regular expression?

http://codereview.chromium.org/3435012/diff/1/3
File benchmarks/regexp.js (right):

http://codereview.chromium.org/3435012/diff/1/3#newcode1
benchmarks/regexp.js:1: // Copyright 2009-2010 the V8 project authors. All
rights reserved.
We usually just update to 2010. No need for the range.

http://codereview.chromium.org/3435012/diff/1/3#newcode36
benchmarks/regexp.js:36: // affect how the regexps match their input. Finally
the strings are
Double space before "Finally" to be consistent with the style in the rest of the
comment.

http://codereview.chromium.org/3435012/diff/1/3#newcode40
benchmarks/regexp.js:40: var RegExp = new BenchmarkSuite('RegExp', 910985, [
I take it that you've measured that these changes do not influence the
performance on the reference platform?

http://codereview.chromium.org/3435012/diff/1/3#newcode59
benchmarks/regexp.js:59: function scrambleString(str, n) {
This seems like a weird name - how about computeInputVariants? Maybe also add a
comment that explains how the variants are generated?

http://codereview.chromium.org/3435012/diff/1/3#newcode60
benchmarks/regexp.js:60: var stringarray = [ str ];
stringarray => variants (to capture more meaning)

http://codereview.chromium.org/3435012/diff/1/3#newcode62
benchmarks/regexp.js:62: pos = Math.floor(Math.random() * str.length);
Declare your local variables with var (pos, chr).

http://codereview.chromium.org/3435012/diff/1/3#newcode302
benchmarks/regexp.js:302: var s57a = scrambleString('fryrpgrq', 40);
It's a bit annoying that the iteration count (40 in this case) is repeated -
first at the call to scrambleString and later at the for loop that iterates over
s57a. Maybe the scrambleString should really return an object that encapsulates
the iteration count too?

http://codereview.chromium.org/3435012/diff/1/3#newcode1743
benchmarks/regexp.js:1743: function runRegExpBenchmark() {
Maybe just rename this to run? It seems like the term RegExpBenchmark is
repeated enough. At the call sites it would still be regexpBenchmark.run() which
looks nice.

http://codereview.chromium.org/3435012/diff/1/3#newcode1760
benchmarks/regexp.js:1760: this.runRegExpBenchmark = runRegExpBenchmark;
this.run = run;

http://codereview.chromium.org/3435012/diff/1/2
File benchmarks/revisions.html (right):

http://codereview.chromium.org/3435012/diff/1/2#newcode30
benchmarks/revisions.html:30: benchmark to exercise the regexp engine on
different input strings.</p>
regexp => regular expression

[sandholm, 2010-09-20 10:32:58]

http://codereview.chromium.org/3435012/diff/1/5
File benchmarks/README.txt (right):

http://codereview.chromium.org/3435012/diff/1/5#newcode74
benchmarks/README.txt:74: benchmark to exercise the regexp engine on different
input strings.
On 2010/09/19 12:45:17, Kasper Lund wrote:
> regexp => regular expression?

Done.

http://codereview.chromium.org/3435012/diff/1/3
File benchmarks/regexp.js (right):

http://codereview.chromium.org/3435012/diff/1/3#newcode1
benchmarks/regexp.js:1: // Copyright 2009-2010 the V8 project authors. All
rights reserved.
On 2010/09/19 12:45:17, Kasper Lund wrote:
> We usually just update to 2010. No need for the range.

Done. 
There seem to be as many ranges as non-ranges in our copyright headers though.

http://codereview.chromium.org/3435012/diff/1/3#newcode36
benchmarks/regexp.js:36: // affect how the regexps match their input. Finally
the strings are
On 2010/09/19 12:45:17, Kasper Lund wrote:
> Double space before "Finally" to be consistent with the style in the rest of
the
> comment.

Done.

http://codereview.chromium.org/3435012/diff/1/3#newcode40
benchmarks/regexp.js:40: var RegExp = new BenchmarkSuite('RegExp', 910985, [
On 2010/09/19 12:45:17, Kasper Lund wrote:
> I take it that you've measured that these changes do not influence the
> performance on the reference platform?

Good catch. Having checked now, I know that they don't.

http://codereview.chromium.org/3435012/diff/1/3#newcode59
benchmarks/regexp.js:59: function scrambleString(str, n) {
On 2010/09/19 12:45:17, Kasper Lund wrote:
> This seems like a weird name - how about computeInputVariants? Maybe also add
a
> comment that explains how the variants are generated?

Done.

http://codereview.chromium.org/3435012/diff/1/3#newcode60
benchmarks/regexp.js:60: var stringarray = [ str ];
On 2010/09/19 12:45:17, Kasper Lund wrote:
> stringarray => variants (to capture more meaning)

Done.

http://codereview.chromium.org/3435012/diff/1/3#newcode62
benchmarks/regexp.js:62: pos = Math.floor(Math.random() * str.length);
On 2010/09/19 12:45:17, Kasper Lund wrote:
> Declare your local variables with var (pos, chr).

Done.

http://codereview.chromium.org/3435012/diff/1/3#newcode302
benchmarks/regexp.js:302: var s57a = scrambleString('fryrpgrq', 40);
On 2010/09/19 12:45:17, Kasper Lund wrote:
> It's a bit annoying that the iteration count (40 in this case) is repeated -
> first at the call to scrambleString and later at the for loop that iterates
over
> s57a. Maybe the scrambleString should really return an object that
encapsulates
> the iteration count too?

I thought about having the number of iterations in a less redundant manner. I am
reusing the same variant array for loops of different sizes though.

http://codereview.chromium.org/3435012/diff/1/3#newcode1743
benchmarks/regexp.js:1743: function runRegExpBenchmark() {
On 2010/09/19 12:45:17, Kasper Lund wrote:
> Maybe just rename this to run? It seems like the term RegExpBenchmark is
> repeated enough. At the call sites it would still be regexpBenchmark.run()
which
> looks nice.

Done.

http://codereview.chromium.org/3435012/diff/1/3#newcode1760
benchmarks/regexp.js:1760: this.runRegExpBenchmark = runRegExpBenchmark;
On 2010/09/19 12:45:17, Kasper Lund wrote:
> this.run = run;

Done.

http://codereview.chromium.org/3435012/diff/1/2
File benchmarks/revisions.html (right):

http://codereview.chromium.org/3435012/diff/1/2#newcode30
benchmarks/revisions.html:30: benchmark to exercise the regexp engine on
different input strings.</p>
On 2010/09/19 12:45:17, Kasper Lund wrote:
> regexp => regular expression

Done.