Closed Bug 899425 Opened 7 years ago Closed 4 years ago

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

Categories

(Firefox for Android :: Toolbar, defect, minor)

ARM
Android
defect
Not set
minor

Tracking

()

RESOLVED FIXED

People

(Reporter: ckitching, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

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