Closed Bug 899425 Opened 10 years ago Closed 7 years ago

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


(Firefox for Android Graveyard :: Toolbar, defect)

Not set


(Not tracked)



(Reporter: ckitching, Unassigned)




(1 file)

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.
Attachment #782946 - Flags: review?
Assignee: nobody → ckitching
Severity: normal → minor
OS: Linux → Android
Hardware: x86_64 → ARM
Attachment #782946 - Flags: review? → review?(bugmail.mozilla)
Comment on attachment 782946 [details] [diff] [review]

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.
Attachment #782946 - Flags: review?(bugmail.mozilla) → review-
Assignee: chriskitching → nobody
The scissor code in LayerRender was removed in bug 1189921.
Closed: 7 years ago
Depends on: 1189921
Resolution: --- → FIXED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.