Closed Bug 921808 Opened 7 years ago Closed 7 years ago

unnecessary style changes trigger reflow/paint during load


(Firefox OS Graveyard :: Gaia::Contacts, defect, P1)

Gonk (Firefox OS)


(blocking-b2g:koi+, b2g-v1.2 fixed)

1.2 C2(Oct11)
blocking-b2g koi+
Tracking Status
b2g-v1.2 --- fixed


(Reporter: bkelly, Assigned: bkelly)


(Depends on 1 open bug)


(Keywords: perf, Whiteboard: [c= p=1 s=2013.10.04 u=1.2])


(1 file)

As can be seen in the following profile at time 17742 there is an additional paint after the list is initialized:

Part of this is due to the dynamic group list creation (bug 921683), but some of it is also caused by the checkCancelableActivity() function in contacts.js.

This function effectively does this:

      appTitleElement.textContent = _('contacts');

These writes to the DOM are unnecessary in this case because the static index.html already specifies these classes and text content.

It would be nice if gecko recognized that no change has occurred, but unfortunately it does not.  For example, see bug 845205 where removing a non-existent style triggers a reflow.

Therefore, we can avoid about 25ms of reflow/paint work if we simply check the state of the classList and textContent before making the changes.

Pull request to come.
This pull request adds some helper routines to check the class state before setting/removing the class.  Annoying, but effective in this case.  The patch also checks the textContent as well before setting.

Profile before at time 17742:

Profile after at time 13357:

These show the reflow/paint is reduced from 50ms to 25ms.  I hope to remove this reflow/paint event completely once bug 921683 is implemented.
Attachment #811632 - Flags: review?(jmcf)
Attachment #811632 - Attachment mime type: text/plain → text/html
Nom koi? because it blocks a koi+.
blocking-b2g: --- → koi?
Comment on attachment 811632 [details]
Pull request at

good work Ben.


PS: Before landing, please check unit tests are Travis seem to be complaining
Attachment #811632 - Flags: review?(jmcf) → review+
Thanks for the review!  The tests were an unrelated intermittent failure in email.

I'm going to hold off on landing for a little bit to see if I can fix this in gecko.  It *looks* like it might be an easy fix, but you never know with the style system.  If it becomes too complicated or triggers mysterious tbpl errors I will proceed with this patch.
Depends on: 922677
So I spent a day or so investigating if the classList part of this patch could be fixed in gecko in bug 922677.  The conclusion I came away from that is that gecko already adequately handles no-op classList add()/remove().  My short-circuit gave some improvement on a micro-benchmark, but that was simply due to avoiding code which is necessary to fire mutation handlers if present.  Since the spec requires mutation handlers to fire in that case we can't use the short-circuit.  In any case, it wasn't causing a paint.

So I looked closer at the textContent assignment.  I now believe this is the only part of this code that is causing a repaint here.  Since the profile has so much going on I decided to verify my belief about the paint a second way.

First, I disabled the hwc by commenting out the block here as suggested by :BenWa:

With the hwc disabled I could then use the "paint flashing" pref in the settings app.  This shows that the the 'Contacts' title is repainted when textContent is re-assigned with the same value.

At a minimum I'm going to rework the pull request to only protect the textContent assignment.
:bz points out that fixing the textContent assignment repaint in gecko would be difficult and has been discussed before.  See bug 725221.

I'll proceed with the app side patch given current priorities.
Depends on: 725221
Since the revised patch was essentially the same as previously reviewed I carried for the r+.

Closed: 7 years ago
Resolution: --- → FIXED
blocking-b2g: koi? → koi+
Priority: -- → P1
Whiteboard: [c= p=1 s= u=] → [c= p=1 s=2013.10.04 u=1.2]
Uplifted 24234de42e9418e5cc602133f482dee7524b887f to:
v1.2: 44fd89ee80a16c1416c72bc83ed08b12340d87d5
Target Milestone: --- → 1.2 C2(Oct11)
You need to log in before you can comment on or make changes to this bug.