Create Task
Maniphest T14757

Possible performance improvements for applet loading
Closed, ResolvedPublic
Actions

Description

While doing some profiling on Plasma startup I noticed that a significant amount of time is spent loading applets (who would have thought :)), in particular Plasma::PluginLoader::loadApplet().

To make profiling of that easier I made a minimal test case based on an applet set observed in my regular session: https://invent.kde.org/nicolasfella/appletbenchmark

Based on profiling that I have some ideas for performance improvements:

A significant amount of time is spent on finding the C++ plugins that some applets have. This is effectively done using KPluginLoader::findPluginsById, which involves looping over all plugins. To speed that up a cache is maintained. However building the cache is itself quite expensive.

A better solution would be to have a way to determine the plugin file name (or lack thereof) from the KPackage metadata. For KCMs we have a X-KDE-Library key in the metadata, I would suggest we do the same here. This way we can skip the costly plugin discovery and feed the file name directly into KPluginLoader. However we would need to keep compatibility for existing applets until Plasma 6.

Another thing that's quite prominent in the profile is KPluginMetaData::fromDesktopFile(). This is due to the fact that applets installed using plasma_install_bundled_package seem to have no metadata.json so the (costly) conversion needs to happen at runtime. See https://mail.kde.org/pipermail/plasma-devel/2021-July/120525.html for more.

Related Objects

alex added a subscriber: alex.Jul 29 2021, 2:22 PM

Another thing that's quite prominent in the profile is KPluginMetaData::fromDesktoFile()

That is also touched in T14558. I will continue working on that.

When looking at the code you see

auto plugins = d->plasmoidCache.findPluginsById(name, {PluginLoaderPrivate::s_plasmoidsPluginDir, {}});

And the same thing for the parent plugin. So we are searching in the plugin root, which contains a ton of files. I agree with specifying if the applet has a C++ plugin or not.

alex added a comment.Jul 29 2021, 3:14 PM

For KCMs we have a X-KDE-Library key in the metadata, I would suggest we do the same here.

The issue is that this key gets removed when the file is converted to JSON.

Usually this makes sense, because the JSON is embedded in the plugin. In case we convert the deskop files to JSON, as proposed in T14564 we could just as easily add it back.

nicolasfella updated the task description. (Show Details)Jul 30 2021, 1:46 PM

plasma-pa, plasma-nm and kdeconnect have been ported to have json package metadata, so the desktop->json conversion is not an issue any more on my system

nicolasfella added a comment.Jul 30 2021, 1:52 PM

Something that's quite prominent in KPackage::PackageLoader::loadPackage() is loadPackageStructure. This is because it iterates over all plugins in "kpackage/packagestructures" to find the right plugin. If we could infer the plugin name from the package type that would help speed this up. We could for example assume that for the package structure "Plasma/Applet" the plugin file is "plasma_applet.so". This still would need compatibility code for Plasma 5 though

alex added a comment.EditedJul 31 2021, 6:48 AM

We could for example assume that for the package structure "Plasma/Applet" the plugin file is "plasma_applet.so".

This makes sense to me to speed it up, however I am not sure we should enforce this implicit/magic behavior.
I think we could try to look for a plugin by the constructed name and if it does not match we search metadata. Ideally with a warning for the developers.

And yeah - in the appstream generation also all available package structures are queried. I already deprecated that, but the usage is implicit so one can't port away from it.

alex claimed this task.EditedJul 31 2021, 6:58 PM

I will work on the KPackage bits. I have touched the related code recently and a familiar with it.

Conceptually this is very similar to T14499, with the difference that we need to do a little string magic ;)

alex added a comment.Aug 6 2021, 2:46 PM
In T14757#261124, @alex wrote:

For KCMs we have a X-KDE-Library key in the metadata, I would suggest we do the same here.

The issue is that this key gets removed when the file is converted to JSON.

Usually this makes sense, because the JSON is embedded in the plugin. In case we convert the deskop files to JSON, as proposed in T14564 we could just as easily add it back.

@nicolasfella How about we settle for X-KDE-Applet-Library? Then we don't get unexpected results from the desktoptojson conversion and emphasize that it is Applet specific.
This is IMHO a good idea, because the usecase of a KPluginMetaData pointing to a cpp plugin is rather rare (except for the KCM stuff).

In case the value is set & not empty we load the lib, if it is set & empty we know that there is no lib to load.

nicolasfella added a comment.Aug 6 2021, 2:47 PM

I don't particularly care about the exact name, so fine with me

alex added a comment.Aug 7 2021, 10:40 AM

I did some benchmarking/debugging and there are some other performance issues in kpackage.

Fixing those is blocking for the property, otherwise we could even worsen performance because it triggers those cases.

But https://invent.kde.org/frameworks/kpackage/-/merge_requests/23 should already be an improvement.

volkov added a subscriber: volkov.Sep 25 2021, 10:59 PM
nicolasfella added a comment.Apr 30 2022, 4:45 PM

@alex do you think there's anything left to do here?

alex added a comment.Apr 30 2022, 6:42 PM

Only some cleanups in the json files, will take care of that in the next week.

alex closed this task as Resolved.EditedMay 29 2022, 3:13 PM

With the package structures being cleaned up, this task is now resolved.

We no longer query the plugin root for applets: https://invent.kde.org/frameworks/plasma-framework/-/merge_requests/352

We do not embed json metadata in the applets when we can use the KPackage metadata:

With this change, we can load the applets by path in KF6 and thus do not need to iterate over all the applets. Unlike the suggestion with the extra key, we would derive the applet path by the plugin id of the metadata.json file.

The KPackage structures can get loaded by name without having to query all the plugins: https://invent.kde.org/frameworks/kpackage/-/merge_requests/23
And the KPluginMetaData object is copied when a KPackage::Package object is copied to avoid unneeded re-reading/re-parsing of the json: https://invent.kde.org/frameworks/kpackage/-/merge_requests/28

The kpackage structure cleanups: