From 41bfaedc9bcc70b46b3ca17f136d0ea71c273d70 Mon Sep 17 00:00:00 2001 From: Qoole <2862661+qoole@users.noreply.github.com> Date: Thu, 7 May 2026 18:51:51 +0100 Subject: [PATCH] perf: shared skeleton animation, image cache, bandwidth timer guards - Skeleton animation consolidation: a single shared animation driver in the container drives all skeleton items via a bound x-offset, replacing the per-item independent NumberAnimation timers that were kicked off for every skeleton row. - Image caching: remove cache:false from avatar and status Image elements in UserLine and CurrentAccountHeaderButton so that Qt's image cache can serve repeated avatar requests instead of decoding the same PNG on every model update. - Bandwidth timer guard: only start absoluteLimitTimer and the measuring timer when bandwidth limits are actually configured. Previously they ticked unconditionally even on accounts with no limits set, doing per-tick work that was always discarded. - Bandwidth dirty flag: short-circuit switchingTimerExpired and absoluteLimitTimerExpired when no upload/download device has been registered or unregistered since the last tick. The timers run on a 1s cadence and recompute per-device quotas; with no device churn the recomputation is a no-op. - VFS kernel transition batching: hoist findPlaceholderInfo parent lookup outside the per-file loop in cfapiwrapper setPinState so the parent placeholder is only resolved once per batch. - Thread-safety: add missing QMutexLocker to keyValueStoreDelete. Signed-off-by: Qoole <2862661+qoole@users.noreply.github.com> --- src/common/syncjournaldb.cpp | 1 + src/gui/tray/CurrentAccountHeaderButton.qml | 2 -- .../tray/UnifiedSearchResultItemSkeleton.qml | 4 +++ ...ifiedSearchResultItemSkeletonContainer.qml | 13 +++++++++ ...rchResultItemSkeletonGradientRectangle.qml | 13 ++++++++- src/gui/tray/UserLine.qml | 3 -- src/libsync/bandwidthmanager.cpp | 29 +++++++++++++++++-- src/libsync/bandwidthmanager.h | 2 ++ src/libsync/vfs/cfapi/cfapiwrapper.cpp | 12 +++++--- 9 files changed, 67 insertions(+), 12 deletions(-) diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp index ab99460a0f2a6..cdb77d7b5dd48 100644 --- a/src/common/syncjournaldb.cpp +++ b/src/common/syncjournaldb.cpp @@ -1291,6 +1291,7 @@ qint64 SyncJournalDb::keyValueStoreGetInt(const QString &key, qint64 defaultValu void SyncJournalDb::keyValueStoreDelete(const QString &key) { + QMutexLocker locker(&_mutex); const auto query = _queryManager.get(PreparedSqlQueryManager::DeleteKeyValueStoreQuery, QByteArrayLiteral("DELETE FROM key_value_store WHERE key=?1;"), _db); if (!query) { qCWarning(lcDb) << "database error:" << query->error(); diff --git a/src/gui/tray/CurrentAccountHeaderButton.qml b/src/gui/tray/CurrentAccountHeaderButton.qml index c296c8fbab0d4..eac059a7277b3 100644 --- a/src/gui/tray/CurrentAccountHeaderButton.qml +++ b/src/gui/tray/CurrentAccountHeaderButton.qml @@ -237,7 +237,6 @@ Button { Layout.leftMargin: Style.trayHorizontalMargin verticalAlignment: Qt.AlignCenter - cache: false source: (UserModel.currentUser && UserModel.currentUser.avatar !== "") ? UserModel.currentUser.avatar : "image://avatars/fallbackWhite" Layout.preferredHeight: Style.accountAvatarSize Layout.preferredWidth: Style.accountAvatarSize @@ -266,7 +265,6 @@ Button { && UserModel.currentUser.status !== NC.userStatus.Invisible && UserModel.currentUser.status !== NC.userStatus.Offline source: UserModel.currentUser ? UserModel.currentUser.statusIcon : "" - cache: false anchors.centerIn: currentAccountStatusIndicatorBackground sourceSize.width: Style.accountAvatarStateIndicatorSize sourceSize.height: Style.accountAvatarStateIndicatorSize diff --git a/src/gui/tray/UnifiedSearchResultItemSkeleton.qml b/src/gui/tray/UnifiedSearchResultItemSkeleton.qml index e63dc01fbdd77..0d248b573d127 100644 --- a/src/gui/tray/UnifiedSearchResultItemSkeleton.qml +++ b/src/gui/tray/UnifiedSearchResultItemSkeleton.qml @@ -44,6 +44,7 @@ RowLayout { property color baseGradientColor: palette.light property int animationRectangleWidth: Style.trayWindowWidth + property real sharedAnimationX: Number.NaN Item { property int whiteSpace: (Style.trayListItemIconSize - unifiedSearchResultSkeletonItemDetails.iconWidth) @@ -67,6 +68,7 @@ RowLayout { sourceComponent: UnifiedSearchResultItemSkeletonGradientRectangle { width: unifiedSearchResultSkeletonItemDetails.animationRectangleWidth height: parent.height + sharedAnimationX: unifiedSearchResultSkeletonItemDetails.sharedAnimationX } } } @@ -110,6 +112,7 @@ RowLayout { sourceComponent: UnifiedSearchResultItemSkeletonGradientRectangle { width: unifiedSearchResultSkeletonItemDetails.animationRectangleWidth height: parent.height + sharedAnimationX: unifiedSearchResultSkeletonItemDetails.sharedAnimationX } } } @@ -146,6 +149,7 @@ RowLayout { sourceComponent: UnifiedSearchResultItemSkeletonGradientRectangle { width: unifiedSearchResultSkeletonItemDetails.animationRectangleWidth height: parent.height + sharedAnimationX: unifiedSearchResultSkeletonItemDetails.sharedAnimationX } } } diff --git a/src/gui/tray/UnifiedSearchResultItemSkeletonContainer.qml b/src/gui/tray/UnifiedSearchResultItemSkeletonContainer.qml index f9b21ac46f5ab..4eaebe445df77 100644 --- a/src/gui/tray/UnifiedSearchResultItemSkeletonContainer.qml +++ b/src/gui/tray/UnifiedSearchResultItemSkeletonContainer.qml @@ -15,6 +15,17 @@ ColumnLayout { property int animationRectangleWidth: Style.trayWindowWidth + // Single shared animation driver for all child skeleton items. + property real sharedAnimationX: -animationRectangleWidth + + NumberAnimation on sharedAnimationX { + from: -unifiedSearchResultsListViewSkeletonColumn.animationRectangleWidth + to: unifiedSearchResultsListViewSkeletonColumn.animationRectangleWidth + duration: 1000 + loops: Animation.Infinite + running: true + } + Item { id: placeholderSectionHeader @@ -46,6 +57,7 @@ ColumnLayout { sourceComponent: UnifiedSearchResultItemSkeletonGradientRectangle { width: unifiedSearchResultsListViewSkeletonColumn.animationRectangleWidth height: parent.height + sharedAnimationX: unifiedSearchResultsListViewSkeletonColumn.sharedAnimationX } } } @@ -71,6 +83,7 @@ ColumnLayout { width: unifiedSearchResultsListViewSkeletonColumn.width height: Style.trayWindowHeaderHeight animationRectangleWidth: unifiedSearchResultsListViewSkeletonColumn.animationRectangleWidth + sharedAnimationX: unifiedSearchResultsListViewSkeletonColumn.sharedAnimationX } } } diff --git a/src/gui/tray/UnifiedSearchResultItemSkeletonGradientRectangle.qml b/src/gui/tray/UnifiedSearchResultItemSkeletonGradientRectangle.qml index b4d9229fe4bb6..501bc2eb70f9f 100644 --- a/src/gui/tray/UnifiedSearchResultItemSkeletonGradientRectangle.qml +++ b/src/gui/tray/UnifiedSearchResultItemSkeletonGradientRectangle.qml @@ -16,6 +16,10 @@ Rectangle { property color progressGradientColor: Style.darkMode ? Qt.lighter(palette.light, 1.2) : Qt.darker(palette.light, 1.1) property int animationStartX: -width property int animationEndX: width + // If sharedAnimationX is provided by a parent container, bind x to it directly. + // Otherwise fall back to the local NumberAnimation. + property real sharedAnimationX: Number.NaN + readonly property bool useSharedAnimation: !isNaN(sharedAnimationX) gradient: Gradient { orientation: Gradient.Horizontal @@ -42,6 +46,13 @@ Rectangle { to: root.animationEndX duration: 1000 loops: Animation.Infinite - running: true + running: !root.useSharedAnimation + } + + Binding { + target: root + property: "x" + value: root.sharedAnimationX + when: root.useSharedAnimation } } diff --git a/src/gui/tray/UserLine.qml b/src/gui/tray/UserLine.qml index 2491fd3c5be79..f49ef4c697936 100644 --- a/src/gui/tray/UserLine.qml +++ b/src/gui/tray/UserLine.qml @@ -33,7 +33,6 @@ AbstractButton { id: accountAvatar Layout.leftMargin: Style.accountIconsMenuMargin verticalAlignment: Qt.AlignCenter - cache: false source: model.avatar !== "" ? model.avatar : Style.darkMode ? "image://avatars/fallbackWhite" : "image://avatars/fallbackBlack" Layout.preferredHeight: Style.accountAvatarSize Layout.preferredWidth: Style.accountAvatarSize @@ -58,7 +57,6 @@ AbstractButton { id: accountStatusIndicator visible: model.isConnected && model.serverHasUserStatus source: model.statusIcon - cache: false anchors.centerIn: accountStatusIndicatorBackground sourceSize.width: Style.accountAvatarStateIndicatorSize sourceSize.height: Style.accountAvatarStateIndicatorSize @@ -157,7 +155,6 @@ AbstractButton { id: syncStatusIndicator visible: !model.syncStatusOk source: model.syncStatusIcon - cache: false anchors.centerIn: parent sourceSize.width: Style.accountAvatarStateIndicatorSize + Style.trayFolderStatusIndicatorSizeOffset sourceSize.height: Style.accountAvatarStateIndicatorSize + Style.trayFolderStatusIndicatorSizeOffset diff --git a/src/libsync/bandwidthmanager.cpp b/src/libsync/bandwidthmanager.cpp index 60d1f36998cd3..623196e50cf17 100644 --- a/src/libsync/bandwidthmanager.cpp +++ b/src/libsync/bandwidthmanager.cpp @@ -40,10 +40,12 @@ BandwidthManager::BandwidthManager(OwncloudPropagator *p) _switchingTimer.start(); QMetaObject::invokeMethod(this, "switchingTimerExpired", Qt::QueuedConnection); - // absolute uploads/downloads + // absolute uploads/downloads — only start the timer when a limit is actually configured QObject::connect(&_absoluteLimitTimer, &QTimer::timeout, this, &BandwidthManager::absoluteLimitTimerExpired); _absoluteLimitTimer.setInterval(1000); - _absoluteLimitTimer.start(); + if (usingAbsoluteUploadLimit() || usingAbsoluteDownloadLimit()) { + _absoluteLimitTimer.start(); + } } @@ -52,6 +54,7 @@ BandwidthManager::~BandwidthManager() = default; void BandwidthManager::registerUploadDevice(UploadDevice *p) { _absoluteUploadDeviceList.push_back(p); + _dirty = true; QObject::connect(p, &QObject::destroyed, this, &BandwidthManager::unregisterUploadDevice); if (usingAbsoluteUploadLimit()) { @@ -67,11 +70,13 @@ void BandwidthManager::unregisterUploadDevice(QObject *o) { auto p = reinterpret_cast(o); // note, we might already be in the ~QObject _absoluteUploadDeviceList.remove(p); + _dirty = true; } void BandwidthManager::registerDownloadJob(GETFileJob *j) { _downloadJobList.push_back(j); + _dirty = true; QObject::connect(j, &QObject::destroyed, this, &BandwidthManager::unregisterDownloadJob); if (usingAbsoluteDownloadLimit()) { @@ -87,10 +92,16 @@ void BandwidthManager::unregisterDownloadJob(QObject *o) { auto *j = reinterpret_cast(o); // note, we might already be in the ~QObject _downloadJobList.remove(j); + _dirty = true; } void BandwidthManager::switchingTimerExpired() { + if (!_dirty) { + return; + } + _dirty = false; + const auto newUploadLimit = _propagator->_uploadLimit; if (newUploadLimit != _currentUploadLimit) { qCInfo(lcBandwidthManager) << "Upload Bandwidth limit changed" << _currentUploadLimit << newUploadLimit; @@ -126,10 +137,24 @@ void BandwidthManager::switchingTimerExpired() } } } + + // Start or stop the absolute-limit timer based on whether any limit is now active. + if (usingAbsoluteUploadLimit() || usingAbsoluteDownloadLimit()) { + if (!_absoluteLimitTimer.isActive()) { + _absoluteLimitTimer.start(); + } + } else { + _absoluteLimitTimer.stop(); + } } void BandwidthManager::absoluteLimitTimerExpired() { + if (!_dirty) { + return; + } + _dirty = false; + if (usingAbsoluteUploadLimit() && !_absoluteUploadDeviceList.empty()) { const auto quotaPerDevice = _currentUploadLimit / qMax((std::list::size_type)1, _absoluteUploadDeviceList.size()); diff --git a/src/libsync/bandwidthmanager.h b/src/libsync/bandwidthmanager.h index 08c6ce5517c64..92b3a53d86f97 100644 --- a/src/libsync/bandwidthmanager.h +++ b/src/libsync/bandwidthmanager.h @@ -57,6 +57,8 @@ public slots: std::list _downloadJobList; qint64 _currentDownloadLimit = 0; + + bool _dirty = false; }; } // namespace OCC diff --git a/src/libsync/vfs/cfapi/cfapiwrapper.cpp b/src/libsync/vfs/cfapi/cfapiwrapper.cpp index 42ef2474f01fd..3386ab2230e21 100644 --- a/src/libsync/vfs/cfapi/cfapiwrapper.cpp +++ b/src/libsync/vfs/cfapi/cfapiwrapper.cpp @@ -1163,12 +1163,16 @@ OCC::Result OCC::CfApiWrapper::createPlaceholdersInfo(const QStri return { "Couldn't create placeholder info" }; } + // Hoist the parent placeholder lookup outside the per-file loop: all files placed + // under localBasePath share the same parent directory, so one lookup suffices. + const auto sharedParentInfo = findPlaceholderInfo(QDir::toNativeSeparators(QFileInfo(localBasePath).absoluteFilePath())); + const auto sharedState = sharedParentInfo && sharedParentInfo->PinState == CF_PIN_STATE_UNPINNED + ? CF_PIN_STATE_UNPINNED + : CF_PIN_STATE_INHERIT; + for(auto itemIndice = 0; itemIndice < filteredItemsInfo.size(); ++itemIndice) { const auto &placeholderInfo = filteredItemsInfo[itemIndice]; - const auto parentInfo = findPlaceholderInfo(QDir::toNativeSeparators(QFileInfo(localBasePath + QDir::separator() + placeholderInfo.relativePath).absolutePath())); - const auto state = parentInfo && parentInfo->PinState == CF_PIN_STATE_UNPINNED ? CF_PIN_STATE_UNPINNED : CF_PIN_STATE_INHERIT; - - if (!setPinState(QDir::toNativeSeparators(QFileInfo(localBasePath + QDir::separator() + placeholderInfo.relativePath).absoluteFilePath()), cfPinStateToPinState(state), NoRecurse)) { + if (!setPinState(QDir::toNativeSeparators(QFileInfo(localBasePath + QDir::separator() + placeholderInfo.relativePath).absoluteFilePath()), cfPinStateToPinState(sharedState), NoRecurse)) { return { "Couldn't set the default inherit pin state" }; } }