[webkit-reviews] review requested: [Bug 19408] Constant folding in parser : [Attachment 22997] Fold only *, +, <<, >>, ~ operators (v4)

[bugzilla-daemon at webkit.org]

Gabor Loki <loki at inf.u-szeged.hu> has asked  for review:
Bug 19408: Constant folding in parser
https://bugs.webkit.org/show_bug.cgi?id=19408

Attachment 22997: Fold only *, +, <<, >>, ~ operators (v4)
https://bugs.webkit.org/attachment.cgi?id=22997&action=edit

------- Additional Comments from Gabor Loki <loki at inf.u-szeged.hu>
(In reply to comment #16)
> As Geoff mentions, you should probably see what happens when you do constant
> folding for division. Why don't you fold the other bitwise operations.

When I have tried to fold divisions I had worse result than without it. The
same happened for other operators. If you like to see those cases and its
results also I will send it separately. 

I have just rechecked division, and now it performs better. So I have included
it into the patch.

> your own shouldBe() function? I don't see any reason why you can't use the
> standard one.

Because of rounding. When I used 'shouldBe' I have for example the following
fail:
FAIL  10.3   -	2.1 should be 8.2. Was 8.200000000000001.

> Other than that, it seems like a good patch. I apologize on behalf of the
> JavaScript team for not reviewing your patch earlier. 

No problem. I have also out of office.


So the attached patch is against rev 35930.

The SunSpider result is:
** TOTAL **:  -  From: 2890.3ms +/- 0.5%  To: 2884.8ms +/- 0.5%

for "*" operator:
  3d-cube:		118.9ms +/- 2.2%    118.4ms +/- 0.6%
  3d-raytrace:		120.4ms +/- 1.1%    121.4ms +/- 1.0%

for "<<" operator:
  crypto-aes:		53.6ms +/- 0.7%     53.7ms +/- 0.6%

for "+" operator:
  crypto-md5:		53.0ms +/- 0.6%     53.1ms +/- 0.4%
  crypto-sha1:		51.7ms +/- 0.9%     51.7ms +/- 0.7%

for "/" operator:
  math-partial-sums:	159.4ms +/- 2.0%    156.7ms +/- 1.2%