ProcessDisplayItems spends about 15ms in Homescreen edit mode when moving an item to change the scrollTop

RESOLVED WONTFIX

Status

Firefox OS
Performance
P3
normal
RESOLVED WONTFIX
3 years ago
4 months ago

People

(Reporter: ethlin, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
When dragging an item in edit mode, I found that the FPS is 30fps and the most time consuming part is ProcessDisplayItems. There are too many items in the Homescreen, so we need to process about 70 items in the ProcessDisplayItems.
(Reporter)

Comment 1

3 years ago
Created attachment 8574484 [details]
Profile result
(Reporter)

Comment 2

3 years ago
Created attachment 8574574 [details]
Profile result

After bug 913443, the profiler is as attached file. The result is similar. The difference is just the large part becomes PopPaintedLayerData from ProcessDisplayItems.
Is there any possible way to improve the PopPaintedLayerData performance when there are a lot of items? Do you think multi-thread for AddPaintedDisplayItem is a good idea?
Attachment #8574484 - Attachment is obsolete: true
Flags: needinfo?(mstange)
(In reply to Ethan Lin[:Ethan] from comment #2)
> Is there any possible way to improve the PopPaintedLayerData performance
> when there are a lot of items?

One fairly simple optimization I can see is to avoid doing duplicate work in the loop that iterates over mAssignedDisplayItems. We could pull the loop into InvalidateForLayerChange and into AddPaintedDisplayItem (making two separate loops out of it). Then we could at least call this only once:

  PaintedDisplayItemLayerUserData* paintedData =
    static_cast<PaintedDisplayItemLayerUserData*>
      (layer->GetUserData(&gPaintedDisplayItemLayerUserData));

instead of for each item. And maybe other caching opportunities would become apparent.

> Do you think multi-thread for
> AddPaintedDisplayItem is a good idea?

I expect it won't be easy to separate the shared state mutating parts from the non-mutating parts of the code. Let's not think about this until we've exhausted the single threaded optimization opportunities.
Flags: needinfo?(mstange)
By the way, it looks like bug 913443 caused a real performance regression, see bug 1141247. Once I have profiles of the regression, I might have more ideas for optimization opportunities.
(Reporter)

Updated

3 years ago
Blocks: 1126646
Created attachment 8579600 [details] [diff] [review]
part 1: pull two hashtable lookups out of a loop
Created attachment 8579601 [details] [diff] [review]
part 2: avoid an array copy by making ClippedDisplayItem and AssignedDisplayItem the same
Created attachment 8579602 [details] [diff] [review]
part 3: clean up arguments of FrameLayerBuilder::AddPaintedDisplayItem
Ethan, can you test whether these patches make a noticeable difference? They didn't help on bug 1141247.
Flags: needinfo?(etlin)
(Reporter)

Comment 10

3 years ago
Markus, thanks for your patches. I tried your patches and the performance has no difference in my case. For now I am also studying the performance issue around here. I will let you know If I have any discovery.
Flags: needinfo?(etlin)

Updated

3 years ago
Priority: -- → P3

Comment 12

4 months ago
Firefox OS is not being worked on
Status: NEW → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.