899425 - LayerRenderer$Frame.transformToScissorRect makes wasteful use of Math.max/min functions

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Toolbar, defect)

Description

[Chris Kitching [:ckitching]]

LayerRenderer$Frame.transformToScissorRect makes use of Math.max/Math.min in a somewhat performance sensitive place. Inlining these calls improves the performance of this function by around 20% (Function calls in Java really are disturbingly expensive...)
Saves us tiny fraction (~0.05%) of our page load time -  four line patch - seemed worth it.

Comment 1

[Chris Kitching [:ckitching]]

Attachment: removeMathMin.patch

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 782946 [details] [diff] [review]
removeMathMin.patch

Review of attachment 782946 [details] [diff] [review]:
-----------------------------------------------------------------

See bug 899424 comment 2. Points 1 and 3 of that comment apply here as well. Point 2 is slightly different:

2) The transformToScissorRect function is called only from setScissorRect, which in turn is called only from drawBackground(), which is called exactly once per frame composited. More notably, it is called on the compositor thread, and so if it's actually on the critical path for page load times on any multi-core device then we have a much bigger problem. I would expect this to make absolutely no difference on a multi-core device with respect to page load time. At best it should reduce compositor jank, but it would only do that if it reduces the frame draw time to below the 16ms threshold when it wasn't before, which seems pretty unlikely to me. It would be interesting to see a CPU-utilization-per-thread graph here to see if this is actually on the critical path.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

The scissor code in LayerRender was removed in bug 1189921.