ElektraInitiative / libelektra

Elektra serves as a universal and secure framework to access configuration settings in a global, hierarchical key database.
https://www.libelektra.org
BSD 3-Clause "New" or "Revised" License
208 stars 123 forks source link

qt-gui: responsiveness #164

Closed markus2330 closed 9 years ago

markus2330 commented 9 years ago

Starting gui on a Intel(R) Xeon(R) CPU E3-1231 v3 @ 3.40GHz takes:

23,50s user 0,77s system 93% cpu 25,948 total

creating a mountpoint takes about the same. Something weird is going on there.

kdb ls alone takes:

$ time kdb ls system | wc -l
1492
kdb ls system  0,06s user 0,00s system 96% cpu 0,070 total
wc -l  0,00s user 0,00s system 0% cpu 0,070 total
0003088 commented 9 years ago

Is 25, 948 the number of keys?

On my system (AMD FX 8350) with ~ 33000 keys:

time bin/qt-gui

real 0m15.467s user 0m12.137s sys 0m2.056s

and

time kdb ls user | wc -l

32805

real 0m0.474s user 0m0.382s sys 0m0.163s

Is this taking too long as well? What would be an acceptable time?

markus2330 commented 9 years ago

Hi,

No, 1492 is the number of system keys, 25,948 is the total time in seconds measured by time(1).

The acceptable time A is:

A=k+q (+10% or 1s)

where:

k .. how long kdb ls needs
q .. how long qt/treeview needs

In your case the ratio total time/k much better, but nevertheless I think that there is a huge constant delay (much more than 10% or 1s).

Maybe the treeview already is the bottleneck, but the issue should be investigated.

Especially the lag after mounting is surprising.

best regards

0003088 commented 9 years ago

The lag after mounting is not surpising since the whole model is populated again. There should be a way around that, by only creating the necessary nodes below /mountpoints. But I do not see a way to initially populate the model except looking at every key in the config.

markus2330 commented 9 years ago

by only creating the necessary nodes below /mountpoints

So you would create a diff and apply the actions the usual way? Sounds very interesting, especially because actions are undoable.

initially populate the model except looking at every key in the config

I created only one issue on purpose, because a solution that covers any reason (e.g. import/export) to (re)build the model is needed. Please investigate if treeview really is the problem. If it is and the problem can be mitigated to a start-up only problem (fixing mounting/import/export by creating diffs), what about a "please wait, the treeview is populating" startup window?

manuelm commented 9 years ago

Just a side note, haven't looked at the code: Is incremental loading possible? This way you only have to create treeview objects for one level at a time. And create the next level after expanding one node, and so on..

markus2330 commented 9 years ago

Hello,

manuel wrote:

Just a side note, haven't looked at the code: Is incremental loading possible? This way you only have to create treeview objects for one level at a time. And create the next level after expanding one node, and so on..

Yeah, from Elektra's point of view it is possible and actually a good idea.

@raffael: What do you think about first loading user, then system,..

0003088 commented 9 years ago

What do you think about first loading user, then system,..

You mean only creating system and user nodes on startup? The nodes need to have children, else they are not expandable in the view. I am very unsure about how incremental loading could be achieved atm.

markus2330 commented 9 years ago

Hello,

0003088 wrote:

You mean only creating system and user nodes on startup? The nodes need to have children, else they are not expandable in the view. I am very unsure about how incremental loading could be achieved atm.

I mean: First show gui w/o any node. Then load all keys below system and update view, then load all keys below user and update view,...

If its complicated a simple "loading" screen for start-up will do, too. Some feedback to the user that qt-gui is loading would be really helpful.

0003088 commented 9 years ago

Added an provisoric loadingscreen in f3de799026. Not fully implemented yet.

0003088 commented 9 years ago

Problem is fixed in 5416cf7.

markus2330 commented 9 years ago

Improved to 0.5 sec constant time.

Tested without any keys and with 328 mountpoints holding 7778 keys.

Very good done!