Differential D10512 Diff 27169 src/plasmaquick/appletquickitem.cpp

Changeset View

Standalone View

View Options

src/plasmaquick/appletquickitem.cpp

Show All 37 Lines
38#include <packageurlinterceptor.h> 38​#include <packageurlinterceptor.h>
39#include <private/package_p.h> 39​#include <private/package_p.h>
40 40​
41namespace PlasmaQuick 41​namespace PlasmaQuick
42{ 42​{
43 43​
44QHash<QObject *, AppletQuickItem *> AppletQuickItemPrivate::s_rootObjects = QHash<QObject *, AppletQuickItem *>(); 44​QHash<QObject *, AppletQuickItem *> AppletQuickItemPrivate::s_rootObjects = QHash<QObject *, AppletQuickItem *>();
45 45​
46​//weight values for the logic for when or if to preload
47​static const int s_defaultPreloadWeight = 50;
broulikUnsubmitted
Not Done

imho using an enum rather than a bunch of static ints is nicer

broulik: imho using an `enum` rather than a bunch of `static int`s is nicer
48​static const int s_defaultLauncherPreloadWeight = 100;
49​static const int s_defaultDatePreloadWeight = 80;
broulikUnsubmitted
Not Done

Are you sure we want to preload the calendar right away? This thing takes forever to open, wouldn't want to have that slow down plasma startup massively.

broulik: Are you sure we want to preload the calendar right away? This thing takes forever to open…
50​static const int s_immediatePreloadWeight = 70;
davidedmundsonUnsubmitted
Not Done

So by default we preload /everything/ then over time your session gets faster as you don't use stuff?

davidedmundson: So by default we preload /everything/ then over time your session gets faster as you don't use…
51​static const int s_delayedPreloadWeight = 10;
52​static const int s_preloadWeightIncrement = 10;
53​
46 54​
47AppletQuickItemPrivate::AppletQuickItemPrivate(Plasma::Applet *a, AppletQuickItem *item) 55​AppletQuickItemPrivate::AppletQuickItemPrivate(Plasma::Applet *a, AppletQuickItem *item)
48 : q(item), 56​ : q(item),
49 switchWidth(-1), 57​ switchWidth(-1),
50 switchHeight(-1), 58​ switchHeight(-1),
51 applet(a), 59​ applet(a),
52 expanded(false), 60​ expanded(false),
53 activationTogglesExpanded(false) 61​ activationTogglesExpanded(false)
54{ 62​{
55} 63​}
broulikUnsubmitted
Not Done

Cache the result in a static to read it only once?

broulik: Cache the result in a static to read it only once?
martAuthorUnsubmitted
Not Done

in a static?

mart: in a static?
56 64​
57void AppletQuickItemPrivate::init() 65​void AppletQuickItemPrivate::init()
broulikUnsubmitted
Not Done

case insensitive?

broulik: case insensitive?
58{ 66​{
59 if (!applet->pluginMetaData().isValid()) { 67​ if (!applet->pluginMetaData().isValid()) {
60 // This `qmlObject` is used in other parts of the code 68​ // This `qmlObject` is used in other parts of the code
61 qmlObject = new KDeclarative::QmlObject(q); 69​ qmlObject = new KDeclarative::QmlObject(q);
62 return; 70​ return;
63 } 71​ }
64 72​
65 qmlObject = new KDeclarative::QmlObjectSharedEngine(q); 73​ qmlObject = new KDeclarative::QmlObjectSharedEngine(q);
66 if (!qmlObject->engine()->urlInterceptor()) { 74​ if (!qmlObject->engine()->urlInterceptor()) {
anthonyfieroniUnsubmitted
Not Done

So Adaptive can a default even environment variable is not setted?
Furthermore you can make a static function to access value

AppletQuickItemPrivate::PreloadPolicy AppletQuickItemPrivate::appletPolicy()
{
    static const PreloadPolicy preloadPolicy = []() -> PreloadPolicy {
        if (qEnvironmentVariableIsSet("PLASMA_PRELOAD_POLICY")) {
            const QString policy = QString::fromUtf8(qgetenv("PLASMA_PRELOAD_POLICY")).toLower();
            if (policy == QStringLiteral("aggressive")) {
                return Aggressive;
            } else if (policy == QStringLiteral("none")) {
                return None;
            }
        }
        return Adaptive;
    }();
    return preloadPolicy;
}
anthonyfieroni: So Adaptive can a default even environment variable is not setted? Furthermore you can make a…
martAuthorUnsubmitted
Not Done

So Adaptive can a default even environment variable is not setted?

yes, adaptive is the default

Furthermore you can make a static function to access value

it's done anyways only once as preloadPolicy is already a static?

mart: > So Adaptive can a default even environment variable is not setted? yes, adaptive is the…
anthonyfieroniUnsubmitted
Not Done

But if PLASMA_PRELOAD_POLICY is not setted s_preloadPolicy stays Uninitialized, no? But since you check against >= Adaptive it's still ok :)

anthonyfieroni: But if PLASMA_PRELOAD_POLICY is not setted s_preloadPolicy stays Uninitialized, no? But since…
broulikUnsubmitted
Not Done

https://cgit.kde.org/plasma-framework.git/commit/?id=fa9d4be5d1663e912e34a5ed581a3bfee159dab8

broulik: https://cgit.kde.org/plasma-framework.git/commit/?id=fa9d4be5d1663e912e34a5ed581a3bfee159dab8
67 PackageUrlInterceptor *interceptor = new PackageUrlInterceptor(qmlObject->engine(), KPackage::Package()); 75​ PackageUrlInterceptor *interceptor = new PackageUrlInterceptor(qmlObject->engine(), KPackage::Package());
68 interceptor->setForcePlasmaStyle(true); 76​ interceptor->setForcePlasmaStyle(true);
69 qmlObject->engine()->setUrlInterceptor(interceptor); 77​ qmlObject->engine()->setUrlInterceptor(interceptor);
70 } 78​ }
71} 79​}
72 80​
81​int AppletQuickItemPrivate::preloadWeight() const
82​{
83​ int defaultWeight;
84​ const QStringList provides(KPluginMetaData::readStringList(applet->pluginMetaData().rawData(), QStringLiteral("X-Plasma-Provides")));
85​
86​ //some applet types we want a bigger weight
87​ if (provides.contains(QStringLiteral("org.kde.plasma.launchermenu"))) {
apolUnsubmitted
Not Done

Isn't X-Plasma-PreloadWeight for that?

apol: Isn't X-Plasma-PreloadWeight for that?
martAuthorUnsubmitted
Not Done

It's to have some default weights without having to populate every desktop file

mart: It's to have some default weights without having to populate every desktop file
88​ defaultWeight = s_defaultLauncherPreloadWeight;
89​ } else if (provides.contains(QStringLiteral("org.kde.plasma.date"))) {
90​ defaultWeight = s_defaultDatePreloadWeight;
91​ } else {
92​ defaultWeight = s_defaultPreloadWeight;
93​ }
94​ //default widgets to be barely preloaded
95​ return qBound(0, applet->config().readEntry(QStringLiteral("PreloadWeight"), qMax(defaultWeight, applet->pluginMetaData().rawData().value(QStringLiteral("X-Plasma-PreloadWeight")).toInt())), 100);
broulikUnsubmitted
Not Done

Could this be split into multiple lines, it's quite hard to follow

broulik: Could this be split into multiple lines, it's quite hard to follow
96​}
97​
73void AppletQuickItemPrivate::connectLayoutAttached(QObject *item) 98​void AppletQuickItemPrivate::connectLayoutAttached(QObject *item)
74{ 99​{
75 QObject *layout = 0; 100​ QObject *layout = 0;
76 101​
77 //Extract the representation's Layout, if any 102​ //Extract the representation's Layout, if any
78 //No Item? 103​ //No Item?
79 if (!item) { 104​ if (!item) {
80 return; 105​ return;
▲ Show 20 Lines • Show All 132 Lines • ▼ Show 20 Line(s)
213QQuickItem *AppletQuickItemPrivate::createFullRepresentationItem() 238​QQuickItem *AppletQuickItemPrivate::createFullRepresentationItem()
214{ 239​{
215 if (fullRepresentationItem) { 240​ if (fullRepresentationItem) {
216 return fullRepresentationItem; 241​ return fullRepresentationItem;
217 } 242​ }
218 243​
219 if (fullRepresentation && fullRepresentation != qmlObject->mainComponent()) { 244​ if (fullRepresentation && fullRepresentation != qmlObject->mainComponent()) {
220 QVariantHash initialProperties; 245​ QVariantHash initialProperties;
221 initialProperties[QStringLiteral("parent")] = QVariant::fromValue(q); 246​ initialProperties[QStringLiteral("parent")] = QVariant();
222 fullRepresentationItem = qobject_cast<QQuickItem*>(qmlObject->createObjectFromComponent(fullRepresentation, QtQml::qmlContext(qmlObject->rootObject()), initialProperties)); 247​ fullRepresentationItem = qobject_cast<QQuickItem*>(qmlObject->createObjectFromComponent(fullRepresentation, QtQml::qmlContext(qmlObject->rootObject()), initialProperties));
223 } else { 248​ } else {
224 fullRepresentation = qmlObject->mainComponent(); 249​ fullRepresentation = qmlObject->mainComponent();
225 fullRepresentationItem = qobject_cast<QQuickItem*>(qmlObject->rootObject()); 250​ fullRepresentationItem = qobject_cast<QQuickItem*>(qmlObject->rootObject());
226 emit q->fullRepresentationChanged(fullRepresentation); 251​ emit q->fullRepresentationChanged(fullRepresentation);
227 } 252​ }
228 253​
229 if (!fullRepresentationItem) { 254​ if (!fullRepresentationItem) {
▲ Show 20 Lines • Show All 363 Lines • ▼ Show 20 Line(s)612 if (!d->compactRepresentationExpander) {
593 } 618​ }
594 619​
595 d->compactRepresentationExpander->loadUrl(compactExpanderUrl); 620​ d->compactRepresentationExpander->loadUrl(compactExpanderUrl);
596 } 621​ }
597 622​
598 d->compactRepresentationCheck(); 623​ d->compactRepresentationCheck();
599 qmlObject()->engine()->rootContext()->setBaseUrl(qmlObject()->source()); 624​ qmlObject()->engine()->rootContext()->setBaseUrl(qmlObject()->source());
600 qmlObject()->engine()->setContextForObject(this, qmlObject()->engine()->rootContext()); 625​ qmlObject()->engine()->setContextForObject(this, qmlObject()->engine()->rootContext());
626​
627​ //if we're expanded we don't care about preloading because it will already be the case
628​ //as well as for containments
629​ if (d->applet->isContainment() ||
630​ d->expanded || d->preferredRepresentation == d->fullRepresentation) {
631​ return;
632​ }
633​
634​ const int preloadWeight = d->preloadWeight();
635​ //decrease weight until we open it again
636​ applet()->config().writeEntry(QStringLiteral("PreloadWeight"), qMax(0, preloadWeight - s_preloadWeightIncrement));
broulikUnsubmitted
Not Done

This should go into a function, there's like three places where a "random" writeEntry is scattered around

broulik: This should go into a function, there's like three places where a "random" `writeEntry` is…
637​
638​ if (preloadWeight >= s_immediatePreloadWeight) {
639​ d->createFullRepresentationItem();
640​ } else if (preloadWeight >= s_delayedPreloadWeight) {
641​ //spread the creation over a random delay between 2 and 10 seconds, to make it look plasma started before, and load everything in the background without big noticeable freezes
642​ QTimer::singleShot(qrand() % ((10000 + 1) - 2000) + 2000, [this]() {
broulikUnsubmitted
Not Done

Add this as context or else it would blow up when the applet is destroyed before the timer fires:

QTimer::singleShot(qrand..., this, [this]() {
broulik: Add `this` as context or else it would blow up when the applet is destroyed before the timer…
643​ d->createFullRepresentationItem();
644​ });
645​ }
601} 646​}
602 647​
603 648​
broulikUnsubmitted
Not Done

Still missing this context:

QTimer::singleShot(delay, this, [this, delay]() {
broulik: Still missing `this` context: ``` QTimer::singleShot(delay, this, [this, delay]() { ```
604Plasma::Package AppletQuickItem::appletPackage() const 649​Plasma::Package AppletQuickItem::appletPackage() const
605{ 650​{
606 return Plasma::Package(d->appletPackage); 651​ return Plasma::Package(d->appletPackage);
607} 652​}
608 653​
609void AppletQuickItem::setAppletPackage(const Plasma::Package &package) 654​void AppletQuickItem::setAppletPackage(const Plasma::Package &package)
610{ 655​{
611 d->appletPackage = package.kPackage(); 656​ d->appletPackage = package.kPackage();
▲ Show 20 Lines • Show All 121 Lines • ▼ Show 20 Line(s)775 if (!d->applet->isContainment() &&
733 d->createCompactRepresentationExpanderItem(); 778​ d->createCompactRepresentationExpanderItem();
734 } 779​ }
735 780​
736 if (d->compactRepresentationExpanderItem) { 781​ if (d->compactRepresentationExpanderItem) {
737 d->compactRepresentationExpanderItem->setProperty("fullRepresentation", QVariant::fromValue<QObject*>(d->createFullRepresentationItem())); 782​ d->compactRepresentationExpanderItem->setProperty("fullRepresentation", QVariant::fromValue<QObject*>(d->createFullRepresentationItem()));
738 } else { 783​ } else {
739 d->fullRepresentationItem->setProperty("parent", QVariant::fromValue<QObject*>(this)); 784​ d->fullRepresentationItem->setProperty("parent", QVariant::fromValue<QObject*>(this));
740 } 785​ }
786​
787​ //increase on open, ignore containments
788​ if (!d->applet->isContainment()) {
789​ applet()->config().writeEntry(QStringLiteral("PreloadWeight"), qMin(d->preloadWeight() + s_preloadWeightIncrement, 100));
davidedmundsonUnsubmitted
Not Done

So if you ever open the any applet slightly more than every alternate login (at any point during that session) over time it'll become blocking preloaded on boot?

Your login to desktop will just get slower and slower until it's ultimately loading everything.

I'm against doing any hacks without prior profiling.

If we find plasma-pa (for example) gains a lot at no cost, great! Add the entry.

If we find plasma-pa loaded in a nanosecond anyway, and takes up 10Mb of RAM to do so, then it's a very silly thing to do.

I'd like to see some numbers for every applet we add it to, I don't think the shell is in a position to do this automatically.

davidedmundson: So if you ever open the any applet slightly more than every alternate login (at any point…
broulikUnsubmitted
Not Done

I was also wondering about dbus-activated applet, from what I can tell whenever I start a media player, the score is reduced, so I'll most likely end up with 0 for media controller immediately.

broulik: I was also wondering about dbus-activated applet, from what I can tell whenever I start a media…
790​ }
741 } 791​ }
742 792​
743 d->expanded = expanded; 793​ d->expanded = expanded;
744 emit expandedChanged(expanded); 794​ emit expandedChanged(expanded);
745} 795​}
746 796​
747bool AppletQuickItem::isActivationTogglesExpanded() const 797​bool AppletQuickItem::isActivationTogglesExpanded() const
748{ 798​{
▲ Show 20 Lines • Show All 73 Lines • Show Last 20 Lines