Bug 26987 – Add fast case for Math.floor(a / b) expressions

• Component: JavaScriptCore
• Status: RESOLVED INVALID

[Zoltan Herczeg]

I have implemented a fast case for Math.floor(a / b) expression.

For this example:

var a = 8
Math.floor(a / 3)

the following code generated:

[   0] enter
[   1] mov               r-15, r0
[   4] mov               r4, r0
[   7] mov               r-15, r1
[  10] op_method_check
[  11] get_by_id         r4, r-10, floor(@id0)
[  19] mov               r5, r-10
  -- new byte codes start here
[  22] jneq_ptr          r4, r-1216737440, 6(->31)
[  26] op_math_floor     r4, r-15, r2, 10(->40)
  -- new byte codes end here
[  31] div               r6, r-15, r2
[  35] call              r4, r4, 2, 15
[  40] end               r4

[Zoltan Herczeg]

Created attachment 32297 [details]
getType() method added for all leaf Nodes

I split the patch into two parts for easier review.

Since my mthod detects Math.floor(a/b) expressions in AST level, I need some kind of node type. I decided to use virtual methods for this task, because emitByteCode has already virtual, so virtual method table has already generated for all nodes.

Perhaps some virtual functions can be eliminated in ExpressionNode using these types.

[Zoltan Herczeg]

Created attachment 32298 [details]
Enable math.floor(a/b) optimization on interpreter

This is the main part of the patch. Well, actually it is just a draft, since I feel you will have several ideas to further improve the code. Furthermore, only interpreter is supported right now. I would like to hear your opinion before I continue the work on JIT.

The following code is executed in 0m1.992s without this patch:

var a = 3
for (var i = 0; i < 10000000; ++i)
    Math.floor(a / -2)

The patch improved the runtime to 0m0.790s, which is 2.5 times faster.

[Zoltan Herczeg]

Created attachment 32299 [details]
SunSpider results

The patch improves the test which actually use the Math.floor(a/b) expressions. On the other hand, there are some slowdowns on other tests.

[Oliver Hunt]

Comment on attachment 32298 [details]
Enable math.floor(a/b) optimization on interpreter

I think the complexity of this patch isn't balanced performance gain.  I recognise that we need to come up with a solution for the problem of the simple builtin function, but i don't believe lexical specialisation is the solution.  It works for call/apply but they have a *huge* overhead on many levels and aren't cached in js, whereas things like the builtin Math functions (and also a number of other builtin functions) are cached in ways that defeat lexical caching, especially in the fast case, and the quantity of calls is likely to be much larger than that of call and apply.

Just to make it clear, i do believe we want to improve performance of builtins, i just don't believe lexical caching is the solution.

[Zoltan Herczeg]

Just closing this bug. Probably there will be no future work on it.