qgis / QGIS

QGIS is a free, open source, cross platform (lin/win/mac) geographical information system (GIS)
https://qgis.org
GNU General Public License v2.0
10.29k stars 2.96k forks source link

QGIS.ini queried/reloaded thousands of times at each operation #31316

Open stelf opened 5 years ago

stelf commented 5 years ago

Description

According to Process Monitor on Windows 10 Build 1903, QGIS 3.8 (and also tested with 3.6) keeps reloading the .INI files and querying network intefraces while layers are not reloaded, but only navigated.

Scenario

An open project with dozen layers loaded entirely from PostGIS, and no folder links in the Browser Pane (removed due to poor performance) results in some 30k queries to the .ini files.

Issue

Such large number of file operations (IMHO redundant and completely irrelevant) by all means leads to degraded performance and as disk operations are slow, and these are effectively OS calls that go deep stacks.

Clue

Ref screenshot: File operations and Network interface

Steps to reproduce

The issue is easily reproducible with the free Process Monitor.

  1. load qgis
  2. load process monitor
  3. filter out everthing out, but qgis-bin.exe's pid
  4. load postgis layers
  5. clean the process monitor (ctrl-x)
  6. scroll the open view
kannes commented 5 years ago

Same on Linux. ~1000 statx and access calls for the ini file in various locations. I think those will be cached by the filesystem or kernel though, so no idea how bad it is.

elpaso commented 5 years ago

It should not be an issue, any decent OS will cache the file in RAM. Besides that I don't see any real alternative: we are using QgsSettings everywhere to get and store settings for everything (user preferences, network settings, DB connections just to cite a few).

nyalldawson commented 5 years ago

@elpaso maybe we could make the global qsettings a static object - there's no need to watch that one for changes post load. That would help somewhat.

elpaso commented 5 years ago

@nyalldawson yeah, but I'd like to have some metrics about the real impact of the issue before making such a change. Would you suggest a QgsApplication::instance() settings() getter and an ApplicationMembers to store it?

stelf commented 5 years ago

@nyalldawson, sounds like a plan, basically any static facility that keeps these settings in-memory is better and on some OS/HDD combinations it may result in performance improvement. Even when settings need be preserved, this can happen in separate thread on per-minute basis.

@elpaso , started monitoring QGIS with various tools as it became nearly unresponsive at some point. although it turned that unresponsiveness was caused by browser pane referencing network resources (in Favorites and mapped drives), the 30k calls for the .INIs may have impact especially with low-memory OS and HDD configs. These grew quickly to 300k after just several minutes of very basic activity.

what other metrics would you need, maybe I can get some?

this was the only noticeable amount of system calls done by the app (apart from the TCP activity to PostGIS).

the interface enumeration also puzzles me, as connection to DB is normally persisted.

NathanW2 commented 5 years ago

I should be quite easy to add a cache in QgsSettings as we override all the method for this kind of thing. I

I'm not sure why Qt is reading the file all the time as I though QSettings cached it in memory and only touched the disk on sync to save the file back, maybe that is the issue though.

In any case, adding a cache for all value() calls should be easy.

On Wed, Aug 21, 2019 at 10:09 PM Gheorghi notifications@github.com wrote:

@nyalldawson https://github.com/nyalldawson, sounds like a plan, basically any static facility that keeps these settings in-memory is better and on some OS/HDD combinations it may result in performance improvement. Even when settings need be preserved, this can happen in separate thread on per-minute basis.

@elpaso https://github.com/elpaso , started monitoring QGIS with various tools as it became nearly unresponsive at some point. although it turned that unresponsiveness was caused by browser pane referencing network resources (in Favorites and mapped drives), the 30k calls for the .INIs may have impact especially with low-memory OS and HDD configs. These grew quickly to 300k after just several minutes of activity.

this was the only noticeable amount of system calls done by the app (apart from the TCP activity to PostGIS).

the interface enumeration also puzzles me, as connection to DB is normally persisted.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/qgis/QGIS/issues/31316?email_source=notifications&email_token=AAC5FXEP3UBVZM5G53V4GQDQFUV53A5CNFSM4IODJSU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4ZOBMI#issuecomment-523428017, or mute the thread https://github.com/notifications/unsubscribe-auth/AAC5FXGLZLYYJU67I7AGVL3QFUV53ANCNFSM4IODJSUQ .

elpaso commented 5 years ago

@NathanW2 I also believe that QSettings caches the values, QSettings::sync() re-synchronize the in-memory data with the data in the file(s). Qt executes ::sync() automatically from time to time. The sync() mechanism is useful when needing to access a QSetting from various processes (not needed when accessing from various threads).

Perhaps there are some sync() calls that we may remove.

stelf commented 3 years ago

@elpaso @NathanW2 so perhaps a feature then, and not a bug? is it to your opinion that the issue be best closed?