Issue 10043

Title
Views with a lot of fields result in a lot of queries when loading them
Priority
feature
Status
in-progress
Nosy list
ced, nicoe, pokoli, reviewbot
Assigned to
nicoe
Keywords
review

Created on 2021-02-01.10:45:14 by nicoe, last changed 7 days ago by reviewbot.

Messages

Author: [hidden] (nicoe) Tryton committer
Date: 2021-04-30.13:43:54
* Cédric Krier  [2021-04-30 10:49 +0200]: 

>> >For me it is a complex code to maintain for
>>
>> I could understand the argument of the complexity for the javascript version
>> (because of the margin, the usage of "getBoundingRect" which is not exactly
>> self-explanatory) but the GTK one is not really complex.
>>
>> But something even more simple could use the height of the screen.
>
>The base height is not really the problem (and yes the screen height
>should probably be used as it is fixed).
>The problem is to know the height of a row before having the row. (I
>really do not like to use another random treeview than the one which
>will display the record).

Well the menu is not a random treeview. It's a treeview which is almost
guaranteed to be there and in which the size of the row will be the minimal
size of the rows (because menu entries are Char fields which can not contain
carriage return).

One issue I have is that the expander could make the row bigger then the usual
ones.

>> Even more so if we take into account the web and the different handheld
>> terminals people could use there.
>>
>> According to https://www.w3schools.com/browsers/browsers_display.asp the
>> resolutions of the screen of people browsing their website are:
>>
>> - Other high resolutions: 52.2%
>> - 1366x768: 24.8%
>> - 1920x1080: 19.2%
>> - Lower: 10.7%
>>
>> (I would have bet that 1920x1080 was the biggest something like 60%, I am far
>> from the truth. It's a bit sad that wikipedia or google do not make their data
>> about this available as they are more mainstream).
>
>So we can take 1080 as reference which correspond to my screen and
>numbers.

The issue is that it's your screen (and mine) but only ⅕ of the screen used by
people in the wild. If we stick only to the numbers found by this survey we
should use 768, but it's not future proof and I infer that people will have 4K
(2160 or even 4320) screens in the not so distant future.

>> >(which could even be a configuration parameter if needed).
>>
>> Why not but it seems a bit too technical to explain to the users.
>
>For me it is for special usage. Like if someone is using the client on a
>unusual screen size. (It is the same as for the search limit).
>
>Or maybe instead of adding a new value, we could still reuse the search
>limit for something like:
>
>limit = int(CONFIG['client.limit'] / min(len(fnames), 10))

Indeed it could work. I prefer this solution to another parameter.

The difficulty is to convey the meaning that the goal is to fill the screen
with the minimal amount of requests.
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-04-30.10:49:03
On 2021-04-30 09:08, Nicolas Évrard wrote:
> * Cédric Krier  [2021-04-30 00:14 +0200]: 
> 
> >For me it is a complex code to maintain for 
> 
> I could understand the argument of the complexity for the javascript version
> (because of the margin, the usage of "getBoundingRect" which is not exactly
> self-explanatory) but the GTK one is not really complex.
> 
> But something even more simple could use the height of the screen.

The base height is not really the problem (and yes the screen height
should probably be used as it is fixed).
The problem is to know the height of a row before having the row. (I
really do not like to use another random treeview than the one which
will display the record).

> >no real guarantee to be correct
> 
> It is best effort, there's no point in determining exactly the right number of
> rows as long as you're a bit but not too much above the number of displayed
> rows.

That's why a fixed number could be a good enough approximation. Maybe 20
is too small and it should be 100 to cover very large screen.

> >and for almost no benefit compared to a fixed minimal value based on common
> >screen sizes
> 
> I don't think there is such a thing as a "common screen size".

Indeed I should not have said common but upper average.

> Even more so if
> we take into account the web and the different handheld terminals people could
> use there.
> 
> According to https://www.w3schools.com/browsers/browsers_display.asp the
> resolutions of the screen of people browsing their website are:
> 
> - Other high resolutions: 52.2%
> - 1366x768: 24.8%
> - 1920x1080: 19.2%
> - Lower: 10.7%
> 
> (I would have bet that 1920x1080 was the biggest something like 60%, I am far
> from the truth. It's a bit sad that wikipedia or google do not make their data
> about this available as they are more mainstream).

So we can take 1080 as reference which correspond to my screen and
numbers.

> >(which could even be a configuration parameter if needed).
> 
> Why not but it seems a bit too technical to explain to the users.

For me it is for special usage. Like if someone is using the client on a
unusual screen size. (It is the same as for the search limit).

Or maybe instead of adding a new value, we could still reuse the search
limit for something like:

limit = int(CONFIG['client.limit'] / min(len(fnames), 10))
Author: [hidden] (nicoe) Tryton committer
Date: 2021-04-30.09:08:10
* Cédric Krier  [2021-04-30 00:14 +0200]: 

>For me it is a complex code to maintain for 

I could understand the argument of the complexity for the javascript version
(because of the margin, the usage of "getBoundingRect" which is not exactly
self-explanatory) but the GTK one is not really complex.

But something even more simple could use the height of the screen.

>no real guarantee to be correct

It is best effort, there's no point in determining exactly the right number of
rows as long as you're a bit but not too much above the number of displayed
rows.

Which I think happen when we using the menu as a base because it has no
buttons, search bar, etc and so the dividend is greater than it would be for
the usual tree view with the search. 

>and for almost no benefit compared to a fixed minimal value based on common
>screen sizes

I don't think there is such a thing as a "common screen size". Even more so if
we take into account the web and the different handheld terminals people could
use there.

According to https://www.w3schools.com/browsers/browsers_display.asp the
resolutions of the screen of people browsing their website are:

- Other high resolutions: 52.2%
- 1366x768: 24.8%
- 1920x1080: 19.2%
- Lower: 10.7%

(I would have bet that 1920x1080 was the biggest something like 60%, I am far
from the truth. It's a bit sad that wikipedia or google do not make their data
about this available as they are more mainstream).

>(which could even be a configuration parameter if needed).

Why not but it seems a bit too technical to explain to the users.
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-04-30.00:14:35

For me it is a complex code to maintain for no real guarantee to be correct and for almost no benefit compared to a fixed minimal value based on common screen sizes (which could even be a configuration parameter if needed).

Author: [hidden] (nicoe) Tryton committer
Date: 2021-04-29.20:44:19

I had the idea to compute the number of rows based on the click to open a new tab on the menu.

That way the minimal number of rows is updated quite often, fore previously opened tabs it doesn't matter but for new tabs it will take into account the eventual new size of the window. Maybe we could plug some event to ensure that it is computed when the window is resized also.

I base the computation on the size of the treeview used for the menu as it's a treeview that we can be almost certain that it will be there. In case it's not realized then I used the default of 20 that ced proposed.

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-02-07.23:46:46

I think the best would be that the minimal is the number of row the screen can display. But it is really not easy to get such value so based on my screen (34 on desktop fullscreen and 20 on web and default desktop), I would go for 20 as minimal limit.

Author: [hidden] (nicoe) Tryton committer
Date: 2021-02-01.10:45:14

Because of the way the limit of the record to load is computed in Tryton (probably sao I haven't checked yet):

limit = int(CONFIG['client.limit'] / len(fnames))

Views with a lot of fields will make a lot of queries to load a big number of records.
We should have an absolute minimum to this limit.

History
Date User Action Args
2021-04-30 18:47:43reviewbotsetmessages: + msg67131
2021-04-30 13:43:54nicoesetmessages: + msg67117
2021-04-30 10:49:03cedsetmessages: + msg67109
2021-04-30 09:08:13nicoesetmessages: + msg67103
2021-04-30 00:14:35cedsetmessages: + msg67101
2021-04-29 20:54:14reviewbotsetmessages: + msg67100
nosy: + reviewbot
2021-04-29 20:44:19nicoesetassignedto: nicoe
messages: + msg67099
status: chatting -> in-progress
2021-04-29 20:39:17nicoesetkeyword: + review
reviews: 336131003
2021-02-07 23:46:46cedsetcomponent: + sao
messages: + msg64358
nosy: + ced
status: unread -> chatting
2021-02-01 11:49:03pokolisetnosy: + pokoli

Showing 10 items. Show all history (warning: this could be VERY long)