D18612: Cache the default KColorScheme configuration
This revision was automatically updated to reflect the committed changes. Closed by commit R265:c0cc6b8a200a: Cache the default KColorScheme configuration (authored by mwolff). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D18612?vs=50541=51212#toc REPOSITORY R265 KConfigWidgets CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18612?vs=50541=51212 REVISION DETAIL https://phabricator.kde.org/D18612 AFFECTED FILES autotests/CMakeLists.txt autotests/kcolorschemetest.cpp src/kcolorscheme.cpp To: mwolff, #kate, #kdevelop, dfaure, broulik Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns
D18612: Cache the default KColorScheme configuration
mwolff added a comment. pushed this now with a proper benchmark too, shows a ~10x performance win when a non-empty PATH is set REPOSITORY R265 KConfigWidgets BRANCH master REVISION DETAIL https://phabricator.kde.org/D18612 To: mwolff, #kate, #kdevelop, dfaure, broulik Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns
D18612: Cache the default KColorScheme configuration
broulik accepted this revision. This revision is now accepted and ready to land. REPOSITORY R265 KConfigWidgets BRANCH master REVISION DETAIL https://phabricator.kde.org/D18612 To: mwolff, #kate, #kdevelop, dfaure, broulik Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns
D18612: Cache the default KColorScheme configuration
mwolff added a comment. sure, but first let's get this in. @broulik or @dfaure care to give your +1? REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D18612 To: mwolff, #kate, #kdevelop, dfaure Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns
D18612: Cache the default KColorScheme configuration
broulik added a comment. I also noticed that on application startup two default color schemes are created: One by plasma-integration set on `QApplication` and the other by `QStyle::standardPalette()` in the widget style. I couldn't figure out a way to avoid this (have QStyle use the default app palette set by p-i or something without dirty hacks like using dynamic properties to communicate that we already have created the proper palette elsewhere). Perhaps this is something you could also profile/investigate :) REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D18612 To: mwolff, #kate, #kdevelop, dfaure Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns
D18612: Cache the default KColorScheme configuration
mwolff added a comment. I see that it's faster when I profile kate/kdev, but I cannot easily write a benchmark for this. It's only noticeable when the KDE_COLOR_SCHEME_PATH variable is set, otherwise the global application config will be used afte rall, which is going to be shared most probably. Any idea how I could construct a valid KDE_COLOR_SCHEME_PATH path to e.g. the breeze color scheme from within a kcolor scheme auto test? REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D18612 To: mwolff, #kate, #kdevelop, dfaure Cc: kde-frameworks-devel, michaelh, ngraham, bruns
D18612: Cache the default KColorScheme configuration
mwolff created this revision. mwolff added reviewers: Kate, KDevelop, dfaure. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. mwolff requested review of this revision. REVISION SUMMARY KDevelop, Kate and probably other applications too, recreate KColorScheme instances repeatedly. This was very costly since we ended up reparsing the internal color scheme configuration file every time - the shared configuration wasn't stored anywhere thus it's refcount dropped to zero after once the KColorScheme was fully constructed. Optimize this apparently common scenario by caching the configuration in a thread_local variable and only open a new configuration when the user changed the application color scheme. REPOSITORY R265 KConfigWidgets BRANCH master REVISION DETAIL https://phabricator.kde.org/D18612 AFFECTED FILES src/kcolorscheme.cpp To: mwolff, #kate, #kdevelop, dfaure Cc: kde-frameworks-devel, michaelh, ngraham, bruns
