nsBlockReflowState::ClearFloats should call nsBlockFrame::WidthToClearPastFloats less

RESOLVED FIXED in mozilla22

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

({perf})

Trunk
mozilla22
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Yesterday I was looking over bent's shoulder at a profile of the B2G contacts app, and noticed we were spending a good bit of time in reflow state construction inside nsBlockFrame::WidthToClearPastFloats.

This is pretty easily avoidable (and was eliminated with a b2g18 version of the patch I'll attach).  Basically, WidthToClearPastFloats is somewhat expensive, and ClearFloats makes no effort to avoid calling it when it's not needed, but easily could do so.
Created attachment 730351 [details] [diff] [review]
Optimize nsBlockReflowState::ClearFloats better, given that nsBlockFrame::WidthToClearPastFloats is somewhat expensive.

This fixes a performance issue that showed up in a profile of the b2g
contacts app (though it's not clear what percentage of the time it was
given the difficulty of understanding output of 'perf').
Attachment #730351 - Flags: review?(dholbert)
Comment on attachment 730351 [details] [diff] [review]
Optimize nsBlockReflowState::ClearFloats better, given that nsBlockFrame::WidthToClearPastFloats is somewhat expensive.

Er, I should write this on top of m-c rather than my patches to bug 25888.
Attachment #730351 - Flags: review?(dholbert)
Created attachment 730353 [details] [diff] [review]
Optimize nsBlockReflowState::ClearFloats better, given that nsBlockFrame::WidthToClearPastFloats is somewhat expensive.

This fixes a performance issue that showed up in a profile of the b2g
contacts app (though it's not clear what percentage of the time it was
given the difficulty of understanding output of 'perf').
Attachment #730353 - Flags: review?(dholbert)
(Assignee)

Updated

6 years ago
Attachment #730351 - Attachment is obsolete: true
It seems to be a 1% win for the contacts app.
Comment on attachment 730353 [details] [diff] [review]
Optimize nsBlockReflowState::ClearFloats better, given that nsBlockFrame::WidthToClearPastFloats is somewhat expensive.

>@@ -940,31 +940,40 @@ nsBlockReflowState::ClearFloats(nscoord 
[...]
>+
>+  if (!mFloatManager->HasAnyFloats()) {
>+    return aY;
>+  }
>+
>   nscoord newY = aY;
> 
>   if (aBreakType != NS_STYLE_CLEAR_NONE) {
>     newY = mFloatManager->ClearFloats(newY, aBreakType, aFlags);
               ^
It looks like this is the only caller of nsFloatManager::ClearFloats().  nsFloatManager::ClearFloats currently has an early-return if !HasAnyFloats() -- but now (with this patch) we will have already checked for that before we even enter nsFloatManager::ClearFloats().

So, the existing check in nsFloatManager::ClearFloats() will now be redundant, so it could perhaps be converted to a precondition / assertion. Doesn't matter much, though; it's also fine as-is.

r=me regardless
Attachment #730353 - Flags: review?(dholbert) → review+
https://hg.mozilla.org/mozilla-central/rev/c46ce2cb5026
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.