Hacker Newsnew | comments | ask | jobs | submitlogin
cheald 1250 days ago | link | parent

Edit (yet again): My initial conclusions were wrong, and it's nearly certainly cheating. Dammit. I hate being wrong in front of people smarter than me. :<

----

I'm running the same benchmark independently right now. Core i7 in a Win7 64-bit install.

For each test, I did 5 runs and averaged them. I increased the number of loops in each test from 25,000 to 250,000 as well.

  Chrome 9.0.576.0
        Stock: 105.28ms
  With "true": 104.44ms

  MS IE 9.0.7930.16406
        Stock: 10.98ms
  With "true": 181.16ms
That's a pretty interesting jump.

Here's a fun little observation:

  Chome: 8.6ms
     IE: 10.9ms

  --- tests/sunspider-0.9.1/math-cordic.js        2010-11-17 00:55:29.000000000 -0700
  +++ tests/sunspider-0.9.1-deadcode/math-cordic.js       2010-11-17 01:10:36.000000000 -0700
  @@ -63,6 +63,7 @@
       TargetAngle = FIXED(28.027);
       CurrAngle = 0;
       for (Step = 0; Step < 12; Step++) {
  +        return;
           var NewX;
           if (TargetAngle > CurrAngle) {
               NewX = X - (Y >> Step);
By returning immediately out of the loop, Chrome's time drops by a factor of 12.1, whereas IE's stays pretty much constant.

I suspect what's happening here is that the IE engine is somehow marking that entire function as deadcode, and thus, not running it; the ~10ms accounts for the time it takes to run that for loop 250k times, but the cordicsincos() code is not being run at all. Ironically, deadcode somewhere in the function causes the engine to NOT throw it all away, and its gets run.

In fact, if we just kill that for loop all together:

  --- tests/sunspider-0.9.1/math-cordic.js        2010-11-17 00:55:29.000000000 -0700
  +++ tests/sunspider-0.9.1-deadcode/math-cordic.js       2010-11-17 01:16:36.000000000 -0700
  @@ -62,20 +62,6 @@

       TargetAngle = FIXED(28.027);
       CurrAngle = 0;
  -    for (Step = 0; Step < 12; Step++) {
  - *snip for brevity*
  -    }
   }
We get the following times:

  Chrome: 8.9ms
      IE: 10.6ms
What I suspect is that the IE engine is seeing "Okay, nothing is returned, and nothing outside of the scope of this function is ever altered", so once it steps into it, it just immediately returns. This is arguably correct behavior! That code is, for all practical purposes, worthless.

If we just move one of the variable references out of function scope (or just remove the var, making it effectively a global variable), IE takes the extra time to run:

  --- tests/sunspider-0.9.1/math-cordic.js        2010-11-17 00:55:29.000000000 -0700
  +++ tests/sunspider-0.9.1-deadcode/math-cordic.js       2010-11-17 01:22:49.000000000 -0700
  @@ -50,11 +50,11 @@
  
  +var CurrAngle;
   function cordicsincos() {
       var X;
       var Y;
       var TargetAngle;
  -    var CurrAngle;
       var Step;

       X = FIXED(AG_CONST);         /* AG_CONST * cos(0) */

  Chrome: 99.9ms
      IE: 217.1ms
Sorry, guys. I like a good IE bash-fest (Hey, it's still slower than V8 when it actually runs the code!) as much as anyone, but I think it's legit here. The benchmark is poorly-conceived, and IE does the right thing with it, though it obviously distorts the scores in their favor. That's a problem with the benchmark, though, not IE.

Edit (like...#14): It could well just be cheating on analysis in this particular case, which I stupidly overlooked. For example, this diff:

  --- tests/sunspider-0.9.1/math-cordic.js        2010-11-17 00:55:29.000000000 -0700
  +++ tests/sunspider-0.9.1-deadcode/math-cordic.js       2010-11-17 02:09:34.000000000 -0700
  @@ -62,7 +62,7 @@

       TargetAngle = FIXED(28.027);
       CurrAngle = 0;
  -    for (Step = 0; Step < 12; Step++) {
  +    for (Step = 12; Step > 0; Step--) {
           var NewX;
           if (TargetAngle > CurrAngle) {
               NewX = X - (Y >> Step);
Results in runtimes of:

  Chrome: 246.8ms
      IE: 956.5ms
Replacing the for with a while also results in long runtimes:

  --- tests/sunspider-0.9.1/math-cordic.js        2010-11-17 00:55:29.000000000 -0700
  +++ tests/sunspider-0.9.1-deadcode/math-cordic.js       2010-11-17 02:12:03.000000000 -0700
  @@ -59,10 +59,11 @@

       X = FIXED(AG_CONST);         /* AG_CONST * cos(0) */
       Y = 0;                       /* AG_CONST * sin(0) */
  +    Step = 0;

       TargetAngle = FIXED(28.027);
       CurrAngle = 0;
  -    for (Step = 0; Step < 12; Step++) {
  +    while(Step < 12) {
           var NewX;
           if (TargetAngle > CurrAngle) {
               NewX = X - (Y >> Step);
  @@ -75,6 +76,7 @@
               X = NewX;
               CurrAngle -= Angles[Step];
           }
  +        Step++;
       }
   }

  Chrome: 103.4ms
      IE: 190.0ms
So, my initial conclusions were wrong. Its dead code analysis is either incredibly narrow, or it was hand-crafted to optimize out that part of the benchmark. Either way it's rubbish.


JoachimSchipper 1249 days ago | link

You missed the point. Yes, dead code analysis may make sense (although a benchmark that includes dead code is probably a bad benchmark...), but this "dead code analysis" fails on utterly trivial variations of the benchmark (http://people.mozilla.com/~sayrer/2010/sunspider/diff1.html, http://people.mozilla.com/~sayrer/2010/sunspider/diff2.html - adding a "true" in the middle breaks it!).

The most likely conclusion is that IE doesn't do any "real" dead code analysis; it just recognizes this particular snippet.

-----

cheald 1249 days ago | link

I think that regrettably, you might be right. It's obviously not just checking for a bytecode match (see my var foo example), but it's doing something hinky. I did a simple pow-and-modulo test with the same assumptions and it didn't optimize it away.

-----

Andrewski 1249 days ago | link

This isn't "regrettable" it's simply par for the course from everybody's favorite tech company.

The only thing they will regret is getting caught, much like any sociopath.

-----

cheald 1249 days ago | link

It's absolutely regrettable. If this was legit, it would mean that the browser would be faster, the user experience would be better, and developers would be another tiny step closer to having an easier time of things when working in IE. I don't feel sorry for Microsoft here, but I'm a web developer, and I want fast, continually-improving browsers to code against.

-----

moconnor 1249 days ago | link

The claim that IE is legit hinges on the diffs causing a valid bug in IE's optimization code (e.g. deadcode inside deadcode prevents the latter being optimized out), versus foul play in the engine (a hard-coded case for this benchmark).

Can you find any variation on the benchmark code that still allows IE to optimize it, or does it only optimize the exact form of the code used in the benchmark suite?

-----

cheald 1249 days ago | link

I changed variable names and declaration order, the number of loops in that inner for loop, and other such things that could possibly change the bytecode (to what effect, I don't know - I'm not a JS VM engineer, obviously) without changing the operations actually performed.

I don't have an explanation for this, though (maybe variable initialization counts as "run up until here"?):

Runs fast (11ms):

  --- tests/sunspider-0.9.1/math-cordic.js        2010-11-17 00:55:29.000000000 -0700
  +++ tests/sunspider-0.9.1-deadcode/math-cordic.js       2010-11-17 01:42:43.000000000 -0700
  @@ -56,12 +56,14 @@
       var TargetAngle;
       var CurrAngle;
       var Step;
  +    var foo;

       X = FIXED(AG_CONST);         /* AG_CONST * cos(0) */
       Y = 0;                       /* AG_CONST * sin(0) */

       TargetAngle = FIXED(28.027);
       CurrAngle = 0;
  +    foo = 1;
       for (Step = 0; Step < 12; Step++) {
           var NewX;
           if (TargetAngle > CurrAngle) {
But if I assign foo after the for loop, it runs slow:

  --- tests/sunspider-0.9.1/math-cordic.js        2010-11-17 00:55:29.000000000 -0700
  +++ tests/sunspider-0.9.1-deadcode/math-cordic.js       2010-11-17 01:43:20.000000000 -0700
  @@ -56,6 +56,7 @@
       var TargetAngle;
       var CurrAngle;
       var Step;
  +    var foo;

       X = FIXED(AG_CONST);         /* AG_CONST * cos(0) */
       Y = 0;                       /* AG_CONST * sin(0) */
  @@ -76,6 +77,7 @@
               CurrAngle -= Angles[Step];
           }
       }
  +    foo = 1;
   }
If I assign foo inside the for loop, it runs fast:

  --- tests/sunspider-0.9.1/math-cordic.js        2010-11-17 00:55:29.000000000 -0700
  +++ tests/sunspider-0.9.1-deadcode/math-cordic.js       2010-11-17 01:44:41.000000000 -0700
  @@ -56,6 +56,7 @@
       var TargetAngle;
       var CurrAngle;
       var Step;
  +    var foo;

       X = FIXED(AG_CONST);         /* AG_CONST * cos(0) */
       Y = 0;                       /* AG_CONST * sin(0) */
  @@ -63,6 +64,7 @@
       TargetAngle = FIXED(28.027);
       CurrAngle = 0;
       for (Step = 0; Step < 12; Step++) {
  +        foo = 1;
           var NewX;
           if (TargetAngle > CurrAngle) {
               NewX = X - (Y >> Step);
Color me boggled.

-----

tel 1249 days ago | link

This reminds me a lot of benchmarking Haskell which does significant dead code analysis and thus was breaking benchmarks. The benchmarks were, of course, modified to do more with the looping code and ensure that all the code paths were run, but the interesting moral is that sometimes artificial benchmarks unsurprisingly don't test what you think they're testing.

-----

ZeroGravitas 1249 days ago | link

The point is whether it does the right thing to all simlar dead code. Or if a human being has made the same analysis as you and added a shortcut that triggers when it sees this exact benchmark code.

The Mozilla guys clearly know this is dead code, do you really want them and every other javascript engine to be adding code targetted at this exact code snippet?

-----

cheald 1249 days ago | link

That's a good point; it appears to be doing legit dead code analysis, but the point still remains that if it's custom-tailored to detect that as dead code when it won't do it in the general case, it's cheating.

I may be eating my hat here, because I just replaced the cordicsincos() with the following:

  function numNumNum() {
    var I;
    var num = 10;
    for (I = 0; I < 10; I++) {
      num = num * num * num * num * num % num;
    }
  }
Using the same benchmarking framework, I get these times:

  Chrome: 849.5ms
      IE: 1226.4ms
That would seem to satisfy all the previous conditions - no leaked scope, no return, no external functions - but it doesn't get optimized away. I'd assumed that by "cheating", it would be hot-swapping that benchmark's bytecode for optimized bytecode, or running a function in C or something, rather than just cheating on the dead code optimization. Bad assumptions make for bad benchmarks!

Witch hunt on!

-----

kenjackson 1249 days ago | link

Cheald, could yu run this test:

function numNumNum() { var I; var num = 10; for (I = 0; I < 10; I++) { num = num + num + num + num + num - num; } }

See if that changes the results?

-----

cheald 1249 days ago | link

Sure. That does, in fact, optimize well!

Edit: I just published my testing setup here: https://github.com/cheald/SunSpider-deadcode

  --- tests/sunspider-0.9.1/math-cordic.js        2010-11-17 00:55:29.000000000 -0700
  +++ tests/sunspider-0.9.1-deadcode/math-cordic.js       2010-11-17 15:08:43.000000000 -0700
  @@ -80,11 +80,15 @@

   ///// End CORDIC

  +function numNumNum() { var I; var num = 10; for (I = 0; I < 10; I++) { num = num + num + num + num + num - num; } }
  +
  +///// End CORDIC
  +
   function cordic( runs ) {
     var start = new Date();

     for ( var i = 0 ; i < runs ; i++ ) {
  -      cordicsincos();
  +      numNumNum();
     }

     var end = new Date();

  Chrome: 19.2ms
      IE: 1.0ms
Curious.

-----

kenjackson 1249 days ago | link

I think this is just fragility, not cheating. I don't know JS super well, but in some languages you might see some rules associated with certain operations and preserving over/under flow exceptions and such.

In any case I think a few things happened here:

1) For whatever reason the "true" statement caused the compiler to think there was a side-effect potential. I suspect the compiler simply didn't know what to do with it, and they hadn't handled 'true;' or 'false;' as standalone statements in their optimizer. I bet if you put 'true;' in the middle of that loop it will break the DCE.

2) The probably don't do liveness analysis. So they can see that a block doesn't change global state, but don't look to see if the proceeding blocks use any of the variables. So if there is any code after a block they assume that they can't DCE that block.

3) '*' and '%' causing problems may be very specific to those operations, and I'm guessing '/' too.

All in all I'd say it is a target incomplete implementation, but not cheating. Based on what I've seen thus far.

-----

kenjackson 1249 days ago | link

And also, thanks for doing the run and posting a link to your setup.

-----

GrandMasterBirt 1249 days ago | link

I have to say, that is some damn good code analysis. The function executes, nothing external happens in the function, nothing is returned from the function: the function is dead code, don't run it.

So yes, while IE9 is technically slower than chrome, it does good stuff with code analysis which chrome should too. Impressive by the way. :)

In the end the benchmark should be augmented to force the function to run.

-----

cheald 1249 days ago | link

It would be good code analysis if it applied in the general case. That was my initial assumption too - that IE is doing the right thing - but the fact that it fails to apply this same analysis in other cases where the same conditions apply (no external scope modified, no returns, etc) makes it feel awfully suspiciously like it's cheating on the analysis of that function in particular.

-----




Lists | RSS | Bookmarklet | Guidelines | FAQ | DMCA | News News | Feature Requests | Bugs | Y Combinator | Apply | Library

Search: