Page MenuHomePhabricator

elm_gengrid: speed up update order calls
ClosedPublic

Authored by bu5hm4n on Mar 15 2019, 6:36 AM.

Details

Summary

update order can be quite expensive, so this here tries to skip it as
often as possible.

ref T7384

Depends on D8366

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 10338
bu5hm4n created this revision.Mar 15 2019, 6:36 AM
bu5hm4n requested review of this revision.Mar 15 2019, 6:36 AM
segfaultxavi requested changes to this revision.Mar 18 2019, 2:47 AM
segfaultxavi added inline comments.
src/lib/elementary/elm_widget_gengrid.h
157

A variable without documentation? inconceivable!

No, seriously, it helps a lot to understand the code.

This revision now requires changes to proceed.Mar 18 2019, 2:47 AM
bu5hm4n updated this revision to Diff 20675.Mar 18 2019, 3:01 AM
bu5hm4n edited the summary of this revision. (Show Details)

make our doc ... ... "cop" happy

Docs are a bit better now :)

Since this is a speedup, is there any way to verify this? any performance test? or just some dragging around that was slow before and is smooth now?

zmike added a subscriber: SanghyeonLee.

@SanghyeonLee can you check this out?

@segfaultxavi you can find the reproduction case and the problem case can be found in the referenced task.

segfaultxavi resigned from this revision.Mar 18 2019, 8:07 AM

OK, thanks. I see there's a mild improvement in scrolling speed. It would be nice if this test output some numbers :)

I won't interfere with this review any longer!

The reasons the performance is still poor are:

  • When the test starts, the whole set of items is realized, then unrealized, then realized again (but only those who are in the viewport)
  • Every time the _elm_gengrid_pan_efl_canvas_group_group_calculate is executed, we walk a list of 5000 items, (which also happens multiple times when we scroll)
  • The list walk in _elm_gengrid_pan_efl_canvas_group_group_calculate does execute a function which calculates and decides all the decisions that we need for placing a item. It looks like it never really cares about if this is even needed or not.

The problem before was that all the 5000K items have been in the focus manager. Now we are down to only those in the viewport.

SanghyeonLee accepted this revision.Mar 24 2019, 9:27 PM

sorry for late review.
on the purpose of removing unnecessary order setup, this looks valid patch.

I think still we can find more needless case that can be skipped,
but that is future work to do I think.

This revision is now accepted and ready to land.Mar 24 2019, 9:27 PM
Closed by commit rEFL04122ec31140: elm_gengrid: speed up update order calls (authored by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain WhyMar 25 2019, 2:54 AM
This revision was automatically updated to reflect the committed changes.