Page MenuHomePhabricator

Elementary: Performance Optimization for Genlist Item Show on launch
AbandonedPublic

Authored by Hermet on Nov 6 2017, 7:38 AM.

Details

Summary

Currently as item processing and item calc happens in linear way from the first item of
the list, if there is a show item in list it will be shown after a signicant
time interval only especially in case of big lists. This patch changes the item
calc order of the queue so that blocks will be assigned to items immediately
for all queued items but the item calc operation will be done starting from the show item.
This solution helps to show the show item immediately when genlist opens.

Signed-off-by: Godly T.Alias <godlytalias@yahoo.co.in>

Test Plan

Elementary Test -> Genlist

Diff Detail

Repository
rEFL core/efl
Branch
arcpatch-D5428
Lint
No Linters Available
Unit
No Unit Test Coverage
godlytalias created this revision.Nov 6 2017, 7:38 AM
godlytalias edited the summary of this revision. (Show Details)Nov 6 2017, 7:48 AM
raster added a comment.Nov 6 2017, 5:32 PM

only if the list is homogenous does genlist know the size of a block... in the case it is not... what is the point of assigning items to blocks when size is still unknown?

In D5428#92578, @raster wrote:

only if the list is homogenous does genlist know the size of a block... in the case it is not... what is the point of assigning items to blocks when size is still unknown?

@raster
As you know, to do item show of show item, we cannot go with the linear way of queue processing. If so we have to
wait till show item is processed which will be a significant amount of time. So the solution to do the item show immediately
is to change the order of item processing happening in queue.
To do so either we can do the complete processing of item starting from the show item so that show item can be shown immediately,
and then to process the items which comes before the show item in the list to be processed in 'prepend' mode, if we
go with such an approach there have to be some amount of code written to change the item relative lists also properly.
Also with such an approach the 'odd-even' UI will fail as we keep prepending item, the shown items will start appearing to change the odd
even color as new items are prepended.
So the solution I found is to just add all items to block initially, so that we will be sure of the order number of item and then only do the item calc,
which is the heavy operation, only to be done in the queue idler. So yes, all items are added in to blocks even when no sizes are known and then it is
flagged as non processed blocks, This is done to maintain the item order properly as we are changing the order of item processings
to show the show item initially. Later the item calc / block calc will happen in the queue idler and once those processings are done we will
come to know about the size of items / blocks.
My aim was to implement a solution with less code changes. If you have some better solution please do share, Thanks.

godlytalias retitled this revision from Elementary: Genlist Show Performance Optimization to Elementary: Performance Optimization for Genlist Item Show on launch.Nov 6 2017, 6:18 PM
SanghyeonLee added a comment.EditedNov 7 2017, 7:52 PM

The idea is actually I have been discussed with hyuksoon choi, but when I applied it in our tizen,
It gave lots of bugs so I abandoned it. I guess there were some mistake of mine, so I hope this one working well.let me check about it.
Thanks to apply this feature :)

but how do you know WHERE the "show item" is without knowing the size od all items before it? imagine a list of 10,000 items. and you want to show item 9,900. you dont know where to jump to because you need all the 9899 before it to be calculated ... ? you dont know where the block is because you dont knwo the size of prior blocks.... :/

In D5428#92922, @raster wrote:

but how do you know WHERE the "show item" is without knowing the size od all items before it? imagine a list of 10,000 items. and you want to show item 9,900. you dont know where to jump to because you need all the 9899 before it to be calculated ... ? you dont know where the block is because you dont knwo the size of prior blocks.... :/

@raster
As per the current solution we dont need to know the size of previous blocks. After assigning blocks for all items we will start the item calculation starting from show item. Show item will be shown in viewport and as soon as other blocks are processed in queue the pan size will change, but the show item will remain in viewport. So basically with this solution we are not following the concept of calculating all previous blocks for calculating exact position of item and scrolling to that position. So we will be processing and showing 9900th item and as other items get processed, pan size keeps changing but the 9900th item will remain in viewport.

how do you ensure the item stays in the viewport? do you keep moving the scroller viewport/position around as you calculate items before.after the show item?

godlytalias added a comment.EditedNov 8 2017, 12:19 AM
In D5428#92926, @raster wrote:

how do you ensure the item stays in the viewport? do you keep moving the scroller viewport/position around as you calculate items before.after the show item?

Currently it remains in viewport with the anchor item logic, also If the check_scroll flag and show_item keeps enabled till queue processing is complete / till any user scroll happens, in the _calc_job we will be doing the interface_region_show for the show item. I couldn't see any other solution than that.

The idea is actually I have been discussed with hyuksoon choi, but when I applied it in our tizen,
It gave lots of bugs so I abandoned it. I guess there were some mistake of mine, so I hope this one working well.let me check about it.
Thanks to apply this feature :)

There is a known issue that as we do the item show immediately genlist height can become 0 causing the ELM_GENLIST_ITEM_SCROLLTO_MIDDLE calculation to fail. We've to defer the show till genlist gets full height for SCROLLTO_MIDDLE to happen properly.

godlytalias updated this revision to Diff 12936.Nov 9 2017, 4:52 AM

Deffering scroll till genlist takes a height to support ELM_GENLIST_SCROLLTO_MIDDLE

godlytalias updated this revision to Diff 12937.Nov 9 2017, 4:57 AM

Removing unwanted changes

Fixes for processing items with relatives

raster requested changes to this revision.Nov 10 2017, 6:29 PM

ok. with all the fixes i have concerns that this might break things. i want you to add in a comprehensive test to elementary test to test this with short and long genlists and all kinds of modes and options. if you have to make the realzing slow with usleeps (e.g. in the icon or label get functions of the item class) to force things to have the list fill very slowly. test showing items at the start, middle, end, other places at the show of the list, test some of the sizing api's that cause genlists to size to their content like elm_genlist_horizontal_mode_set and ELM_LIST_LIMIT and more. test inserting/appending/prepending items while the list still fills and you show an item with show gravity in various states ... you get where i am coming from. test all the things that might break this patch and where it might fail. i am not comfortable that there will be enough testing to know if this breaks anything and how much.

This revision now requires changes to proceed.Nov 10 2017, 6:29 PM

@raster Yes, I'm making test cases for all scenarios and that is how I was able to find those issues, Was not sure whether to push all the scenarios to elementary test, However I will try to cover all the scenarios with the changes and will update elementary test too. Thanks for the review.

yes. definitely want the test cases. preferably in another patch separate to this (link it in as a dependency to this)

Improving the scroll deferring logics and scrolling when item cannot be scrolled to scroll_to position

Block processing improved

raster requested changes to this revision.Nov 21 2017, 6:25 PM

ok. using the new tests. you have bugs. i thought there would be. try this:

append 10000 items (in test) and jump to item 8000. plain or tree test.

it comes up at the right item but now SCROLL up or down while its still filling the list. it jumps up far away from the shown item range immediately. do this scroll before the fill is complete.

This revision now requires changes to proceed.Nov 21 2017, 6:25 PM

going to add a note: tests are in this patch: D5499

@raster
Yes. This is a known limitation with this approach and I was trying some solutions to handle this. As we are not processing items in proper top to down order, and items are getting filled to both top & bottom directions of list from the show item position, So when user is scrolling, the anchor item logic cannot hold and as pan object height is changing, scroll position jumps are happening. I will update patch once some good solution on this is done.

In D5428#94512, @raster wrote:

ok. using the new tests. you have bugs. i thought there would be. try this:

append 10000 items (in test) and jump to item 8000. plain or tree test.

it comes up at the right item but now SCROLL up or down while its still filling the list. it jumps up far away from the shown item range immediately. do this scroll before the fill is complete.

well i see it as a bug. what you might do is come up with a way of having "empty blocks" that have not been filled in/realized yet and padthese between start and "item show" point.

godlytalias added a comment.EditedNov 22 2017, 6:40 AM
In D5428#94545, @raster wrote:

well i see it as a bug. what you might do is come up with a way of having "empty blocks" that have not been filled in/realized yet and padthese between start and "item show" point.

@raster
With empty blocks also, as we won't be able to know the block size before processing items in block, we may end up with pan object size changes during processing of items in queue.
One working solution which I found is pausing processing of items in queue when a user scroll happens so that pan object size and position will not be disturbed and then resume processing when user stops scrolling. But I am not sure whether it is acceptable.

With empty blocks also, as we won't be able to know the block size

With homogeneous lists you can calculate it exactly. But without, you are right, so what you have to do is make some size assumption (like use an average size + group header avg size * number of headers etc.) UNTIL you know the real size. when you have to realize items in a block you can calculate then, so the block you scroll up to or down to can then be figured out. When you do this you MIGHT have to change the block size because the assumption was wrong. When this happens you will have to adjust the viewport view position based on this sizing change too to try keep the view "stable". As long as items need processing you need to handle this case now because you allow people to scroll PAST items that have not been processed yet. just a simple single mouse wheel up will make the view jump somewhere "crazy".

Hermet commandeered this revision.Mar 27 2019, 6:44 PM
Hermet added a reviewer: godlytalias.
Hermet abandoned this revision.Mar 27 2019, 6:44 PM

This is a too old patch, nobody keeps tracking on this anymore.