r19952: performance degradation

On Sun, 2009-12-06 at 13:17 +0300, Andrew Gaydenko wrote:
> Of course, results are gathering after warming JVM up. And, of course, many
> revisions before and after r19952 were tried - historically and during
> bisecting.

By the way, r19952 is:

"fix msil code generation for exception handlers."
https://lampsvn.epfl.ch/trac/scala/changeset/19952

When I saw that commit, I thought it just affected the msil back-end.
Does it also change the code emitted by the JVM back-end?

Best,
Ismael

On Sun, Dec 6, 2009 at 11:26, Ismael Juma <mlists@juma.me.uk> wrote:

On Sun, 2009-12-06 at 13:17 +0300, Andrew Gaydenko wrote:
> Of course, results are gathering after warming JVM up. And, of course, many
> revisions before and after r19952 were tried - historically and during
> bisecting.

By the way, r19952 is:

"fix msil code generation for exception handlers."
https://lampsvn.epfl.ch/trac/scala/changeset/19952

When I saw that commit, I thought it just affected the msil back-end.
Does it also change the code emitted by the JVM back-end?

No, except for one minor change. For a `try`-`catch` with a `finally`, the
exception handler covering the `catch` blocks (which makes sure that the
finalizer gets executed even if an exception is thrown inside a `catch`
handler) now has `Throwable` as type in the exception table, not `null`.

This should not change performance (and of course not the semantics either)
I guess..

On Sunday 06 December 2009 16:10:54 Lukas Rytz wrote:
> No, except for one minor change. For a `try`-`catch` with a `finally`, the
> exception handler covering the `catch` blocks (which makes sure that the
> finalizer gets executed even if an exception is thrown inside a `catch`
> handler) now has `Throwable` as type in the exception table, not `null`.
>
> This should not change performance (and of course not the semantics either)
> I guess..

Lukas,

Thanks for the hint! Have got it. This code:

//---- code start
object R19952 {

val range = 1 to 20000000
val count = 1 to 20

def step {
val started = new java.util.Date
try { range.foldLeft(0)(_+_) } catch { case _ => }
val delta = (new java.util.Date getTime) - started.getTime
println("delta: " + delta)
}

def main(a : Array[String]) : Unit = {
count.foreach(_ => step)
}

}
//---- code end

being started with this script:

//---- scripy start
#!/bin/bash

ORIG_PATH=$PATH

PATH=/wrk/dev/scala/SVN/dists/r19951/bin:$ORIG_PATH
scala -version
rm *.class
scalac R19952.scala
scala R19952

PATH=/wrk/dev/scala/SVN/dists/r19952/bin:$ORIG_PATH
scala -version
rm *.class
scalac R19952.scala
scala R19952
//---- script end

produces this output:

//---- output start
$ ./start.sh
Scala code runner version 2.8.0.r19951-b20091206092628 -- Copyright 2002-2009,
LAMP/EPFL
delta: 544
delta: 520
delta: 506
delta: 506
delta: 507
delta: 507
delta: 507
delta: 498
delta: 508
delta: 509
delta: 507
delta: 505
delta: 506
delta: 503
delta: 502
delta: 510
delta: 506
delta: 506
delta: 502
delta: 509
Scala code runner version 2.8.0.r19952-b20091206030507 -- Copyright 2002-2009,
LAMP/EPFL
delta: 1182
delta: 1395
delta: 1155
delta: 1340
delta: 1342
delta: 1349
delta: 1336
delta: 1328
delta: 1102
delta: 1331
delta: 1350
delta: 1344
delta: 1344
delta: 1344
delta: 1339
delta: 1345
delta: 1350
delta: 1349
delta: 1345
delta: 1350
//---- output end

Andrew

Thanks for trimming it down. I'll have a look.

On Sun, Dec 6, 2009 at 14:59, Andrew Gaydenko <a@gaydenko.com> wrote:

On Sunday 06 December 2009 16:10:54 Lukas Rytz wrote:
> No, except for one minor change. For a `try`-`catch` with a `finally`, the
> exception handler covering the `catch` blocks (which makes sure that the
> finalizer gets executed even if an exception is thrown inside a `catch`
> handler) now has `Throwable` as type in the exception table, not `null`.
>
> This should not change performance (and of course not the semantics either)
> I guess..

Lukas,

Thanks for the hint! Have got it. This code:

//---- code start
object R19952 {

 val range = 1 to 20000000
 val count = 1 to 20

 def step {
   val started = new java.util.Date
   try { range.foldLeft(0)(_+_) } catch { case _ => }
   val delta = (new java.util.Date getTime) - started.getTime
   println("delta: " + delta)
 }

 def main(a : Array[String]) : Unit = {
   count.foreach(_ =>  step)
 }

}
//---- code end

being started with this script:

//---- scripy start
#!/bin/bash

ORIG_PATH=$PATH

PATH=/wrk/dev/scala/SVN/dists/r19951/bin:$ORIG_PATH
scala -version
rm *.class
scalac R19952.scala
scala R19952

PATH=/wrk/dev/scala/SVN/dists/r19952/bin:$ORIG_PATH
scala -version
rm *.class
scalac R19952.scala
scala R19952
//---- script end

produces this output:

//---- output start
$ ./start.sh
Scala code runner version 2.8.0.r19951-b20091206092628 -- Copyright 2002-2009,
LAMP/EPFL
delta: 544
delta: 520
delta: 506
delta: 506
delta: 507
delta: 507
delta: 507
delta: 498
delta: 508
delta: 509
delta: 507
delta: 505
delta: 506
delta: 503
delta: 502
delta: 510
delta: 506
delta: 506
delta: 502
delta: 509
Scala code runner version 2.8.0.r19952-b20091206030507 -- Copyright 2002-2009,
LAMP/EPFL
delta: 1182
delta: 1395
delta: 1155
delta: 1340
delta: 1342
delta: 1349
delta: 1336
delta: 1328
delta: 1102
delta: 1331
delta: 1350
delta: 1344
delta: 1344
delta: 1344
delta: 1339
delta: 1345
delta: 1350
delta: 1349
delta: 1345
delta: 1350
//---- output end

Andrew

On Sun, Dec 06, 2009 at 05:36:20PM +0100, Lukas Rytz wrote:
> Thanks for trimming it down. I'll have a look.

Also, I reproduced the slowdown and verified that this diff alone is
enough to induce it.

- (NoSymbol, expectedType, exhCtx => {
+ (ThrowableClass, expectedType, exhCtx => {

amazing.. this probably means that we can speed up any exception
handler catching `Throwable`.

On Sun, Dec 6, 2009 at 19:14, Paul Phillips <paulp@improving.org> wrote:

On Sun, Dec 06, 2009 at 05:36:20PM +0100, Lukas Rytz wrote:
> Thanks for trimming it down. I'll have a look.

Also, I reproduced the slowdown and verified that this diff alone is
enough to induce it.

-          (NoSymbol, expectedType, exhCtx => {
+          (ThrowableClass, expectedType, exhCtx => {

--
Paul Phillips      | Where there's smoke, there's mirrors!
Future Perfect     |
Empiricist         |
pal, i pill push   |----------* http://www.improving.org/paulp/ *----------

On Sunday 06 December 2009 23:51:09 Lukas Rytz wrote:
> amazing.. this probably means that we can speed up any exception
> handler catching `Throwable`.

so, shortly will be better than ever?

On Sun, Dec 06, 2009 at 09:51:09PM +0100, Lukas Rytz wrote:
> amazing.. this probably means that we can speed up any exception
> handler catching `Throwable`.

I was thinking the same thing - this will turn out to be one of those
performance regressions which ends up leading to a win. We would really
benefit from a regression suite which is specifically looking for
performance backsliding; if we didn't have andrew to catch this one
immediately, I expect it might have remained for a long time.

Cross-posting to the JVM Languages group. You might want to see the
history of this thread as well:

http://old.nabble.com/-scala--r19952%3A-performance-degradation-to266635...

Can anyone from the experts comment on the fact why using null as
exception type ('any' in javap) in a handler instead of Throwable
results in such a big performance win (> 2x) ? Is it a special
performance optimization in the Hotspot VM for finally clauses? Is it
known and documented somewhere?

Thanks,
Johannes

On Sun, Dec 6, 2009 at 9:51 PM, Lukas Rytz wrote:
> amazing.. this probably means that we can speed up any exception
> handler catching `Throwable`.
>
> On Sun, Dec 6, 2009 at 19:14, Paul Phillips wrote:
>>
>> On Sun, Dec 06, 2009 at 05:36:20PM +0100, Lukas Rytz wrote:
>> > Thanks for trimming it down. I'll have a look.
>>
>> Also, I reproduced the slowdown and verified that this diff alone is
>> enough to induce it.
>>
>> -          (NoSymbol, expectedType, exhCtx => {
>> +          (ThrowableClass, expectedType, exhCtx => {
>>
>> --
>> Paul Phillips      | Where there's smoke, there's mirrors!
>> Future Perfect     |
>> Empiricist         |
>> pal, i pill push   |----------* http://www.improving.org/paulp/
>> *----------
>
>

On Sunday 06 December 2009 23:51:09 Lukas Rytz wrote:
> amazing.. this probably means that we can speed up any exception
> handler catching `Throwable`.

Have tried the r20050 fix. Results against r19951 are:

- small test published here - approximately the same
- httpd ApacheBench-marking - 15-20%(!) faster
- text-oriented combinators - 5-7% faster

Great, thanks!

Andrew

On Tue, 2009-12-08 at 14:40 +0300, Andrew Gaydenko wrote:
> On Sunday 06 December 2009 23:51:09 Lukas Rytz wrote:
> > amazing.. this probably means that we can speed up any exception
> > handler catching `Throwable`.
>
> Have tried the r20050 fix. Results against r19951 are:
>
> - small test published here - approximately the same
> - httpd ApacheBench-marking - 15-20%(!) faster
> - text-oriented combinators - 5-7% faster
>
> Great, thanks!

This should win the "most unexpected source of performance improvement"
award for 2009. Amazing.

Thanks for noticing the change and narrowing it down to a single
revision, Andrew. I don't think this would have been found otherwise.

Best,
Ismael