CSS::ProcessRestyles takes 150ms flipping background color of 10,000 elements

RESOLVED DUPLICATE of bug 1493420

Status

()

RESOLVED DUPLICATE of bug 1493420
5 years ago
2 months ago

People

(Reporter: BenWa, Unassigned)

Tracking

Trunk
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 8375180 [details]
tiling-worse-case.html

I'm writing a simple testcase to test worse case GFX tiling performance and I got a total hang. I reduced the test to 10,000 elements. This might not be the most realistic test case but it fells like the style system should be handle this much better so filing this so someone can investigate.

We seem to be spending too much time allocating memory here:
http://people.mozilla.org/~bgirard/cleopatra/#report=af823add5c9302981b3004d0956e428dd150874e
I am surprised we are allocating memory here.  nsStyleBackground has an nsAutoTArray<Layer,1>, so that AppendElement call in the nsStyleBackground constructor shouldn't need any memory allocation.
My mistake we're in the nsStyleBackground copy constructor, but even so, the initialization mLayers(aSource.mLayers) shouldn't need an allocation.
OK so it looks like nsAutoTArray starts off with mHdr->mCapacity == 0 when constructed via a copy constructor, and so the AppendElements call that copies the elements from the other array calls EnsureCapacity and allocates storage.
I would expect from glancing at nsTArray.h that the copy construction of mLayers would result in us using the nsAutoTArray copy constructor, which should then invoke the default constructor of nsAutoArrayBase, which calls Init to initialize our mCapacity.  This doesn't seem to happen.  Someone with a better grasp of our the array templates might be able to say why.
Created attachment 8375225 [details] [diff] [review]
array patch

This avoids the array allocations.  The test is still very slow though.
Without patch applied, I get an average of 515 ms per rAF callback run.  With patch applied, I get an average of 486 ms.  (Average of 50 runs.)
(Reporter)

Comment 7

5 years ago
A step in the right direction at least.
Depends on: 972167

Comment 8

2 months ago
1 second spent in styling : https://perfht.ml/2Nsud7s
Flags: needinfo?(emilio)
Same root cause as bug 1493420.
Status: NEW → RESOLVED
Last Resolved: 2 months ago
Flags: needinfo?(emilio)
Resolution: --- → DUPLICATE
Duplicate of bug: 1493420
You need to log in before you can comment on or make changes to this bug.