From Andrew Lindesay <apl@xxxxxxxxxxxxxx>:
Andrew Lindesay has uploaded this change for review. (
https://review.haiku-os.org/c/haiku/+/2748 ;)
Change subject: HaikuDepot: Performance
......................................................................
HaikuDepot: Performance
This change removes two mistakes I made a long time ago
that caused unnecessarily copying of lists of data. This
fix speeds up the UI alot.
This change also clears data in UI list elements when a
bulk load is requested. It stops clearing otherwise and
instead uses "add" and "remove" operations in the lists
which is OK now because the UI list elements are much
faster than they have been in the past. This removes
the strange clean-and-reload that was visible in the UI
previously.
A threaded package loading system was put in place a long
time ago, but with these performance improvements this
mechanism is no longer necessary; it has been removed to
simplify the code.
Fixes #16012
---
M src/apps/haikudepot/HaikuDepotConstants.h
M src/apps/haikudepot/model/Model.cpp
M src/apps/haikudepot/model/Model.h
M src/apps/haikudepot/server/ServerIconExportUpdateProcess.cpp
M src/apps/haikudepot/server/ServerPkgDataUpdateProcess.cpp
M src/apps/haikudepot/ui/FeaturedPackagesView.cpp
M src/apps/haikudepot/ui/MainWindow.cpp
M src/apps/haikudepot/ui/MainWindow.h
8 files changed, 91 insertions(+), 263 deletions(-)
git pull ssh://git.haiku-os.org:22/haiku refs/changes/48/2748/1
diff --git a/src/apps/haikudepot/HaikuDepotConstants.h
b/src/apps/haikudepot/HaikuDepotConstants.h
index 4b5be61..bb72497 100644
--- a/src/apps/haikudepot/HaikuDepotConstants.h
+++ b/src/apps/haikudepot/HaikuDepotConstants.h
@@ -11,8 +11,6 @@
MSG_PACKAGE_SELECTED = 'pkgs',
MSG_PACKAGE_WORKER_BUSY = 'pkwb',
MSG_PACKAGE_WORKER_IDLE = 'pkwi',
- MSG_ADD_VISIBLE_PACKAGES = 'avpk',
- MSG_UPDATE_SELECTED_PACKAGE = 'uspk',
MSG_CLIENT_TOO_OLD =
'oldc',
MSG_NETWORK_TRANSPORT_ERROR = 'nett',
MSG_SERVER_ERROR =
'svre',
diff --git a/src/apps/haikudepot/model/Model.cpp
b/src/apps/haikudepot/model/Model.cpp
index e2352eb..de427d0 100644
--- a/src/apps/haikudepot/model/Model.cpp
+++ b/src/apps/haikudepot/model/Model.cpp
@@ -353,29 +353,19 @@
}
-PackageList
-Model::CreatePackageList() const
+// TODO; part of a wider change; cope with the package being in more than one
+// depot
+PackageInfoRef
+Model::PackageForName(const BString& name)
{
- // Iterate all packages from all depots.
- // If configured, restrict depot, filter by search terms, status, name
...
- PackageList resultList;
-
- for (int32 i = 0; i < fDepots.CountItems(); i++) {
- const DepotInfo& depot = fDepots.ItemAtFast(i);
-
- if (fDepotFilter.Length() > 0 && fDepotFilter != depot.Name())
- continue;
-
- const PackageList& packages = depot.Packages();
-
- for (int32 j = 0; j < packages.CountItems(); j++) {
- const PackageInfoRef& package = packages.ItemAtFast(j);
- if (MatchesFilter(package))
- resultList.Add(package);
- }
+ DepotList depots = Depots();
+ for (int32 d = 0; d < depots.CountItems(); d++) {
+ const DepotInfo& depot = depots.ItemAtFast(d);
+ int32 packageIndex = depot.PackageIndexByName(name);
+ if (packageIndex >= 0)
+ return depot.Packages().ItemAtFast(packageIndex);
}
-
- return resultList;
+ return PackageInfoRef();
}
@@ -384,6 +374,7 @@
{
return fCategoryFilter->AcceptsPackage(package)
&& fSearchTermsFilter->AcceptsPackage(package)
+ && (fDepotFilter.IsEmpty() || fDepotFilter ==
package->DepotName())
&& (fShowAvailablePackages || package->State() != NONE)
&& (fShowInstalledPackages || package->State() !=
ACTIVATED)
&& (fShowSourcePackages || !is_source_package(package))
diff --git a/src/apps/haikudepot/model/Model.h
b/src/apps/haikudepot/model/Model.h
index 7fd161c..1aa57cc 100644
--- a/src/apps/haikudepot/model/Model.h
+++ b/src/apps/haikudepot/model/Model.h
@@ -77,9 +77,7 @@
bool AddListener(const
ModelListenerRef& listener);
- // !Returns new PackageInfoList from current parameters
- PackageList CreatePackageList()
const;
-
+ PackageInfoRef PackageForName(const BString&
name);
bool MatchesFilter(
const
PackageInfoRef& package) const;
diff --git a/src/apps/haikudepot/server/ServerIconExportUpdateProcess.cpp
b/src/apps/haikudepot/server/ServerIconExportUpdateProcess.cpp
index a8d14ba..d1d59d1 100644
--- a/src/apps/haikudepot/server/ServerIconExportUpdateProcess.cpp
+++ b/src/apps/haikudepot/server/ServerIconExportUpdateProcess.cpp
@@ -147,7 +147,7 @@
printf("[%s] will populate icons for depot [%s]\n",
Name(), depot.Name().String());
status_t result = B_OK;
- PackageList packages = depot.Packages();
+ const PackageList& packages = depot.Packages();
for(int32 j = 0;
(j < packages.CountItems()) && !WasStopped() && (result ==
B_OK);
j++) {
diff --git a/src/apps/haikudepot/server/ServerPkgDataUpdateProcess.cpp
b/src/apps/haikudepot/server/ServerPkgDataUpdateProcess.cpp
index 6e4695a..fc6cc9c 100644
--- a/src/apps/haikudepot/server/ServerPkgDataUpdateProcess.cpp
+++ b/src/apps/haikudepot/server/ServerPkgDataUpdateProcess.cpp
@@ -200,11 +200,11 @@
const DepotInfo* depotInfo = fModel->DepotForName(fDepotName);
if (depotInfo != NULL) {
- BString packageName = *(pkg->Name());
+ const BString packageName = *(pkg->Name());
int32 packageIndex = depotInfo->PackageIndexByName(packageName);
if (-1 != packageIndex) {
- PackageList packages = depotInfo->Packages();
+ const PackageList& packages = depotInfo->Packages();
const PackageInfoRef& packageInfoRef =
packages.ItemAtFast(packageIndex);
diff --git a/src/apps/haikudepot/ui/FeaturedPackagesView.cpp
b/src/apps/haikudepot/ui/FeaturedPackagesView.cpp
index 10427cc..1dd9b0f 100644
--- a/src/apps/haikudepot/ui/FeaturedPackagesView.cpp
+++ b/src/apps/haikudepot/ui/FeaturedPackagesView.cpp
@@ -199,6 +199,10 @@
void Clear()
{
+ for (std::vector<PackageInfoRef>::iterator it =
fPackages.begin();
+ it != fPackages.end(); it++) {
+ (*it)->RemoveListener(fPackageListener);
+ }
fPackages.clear();
fSelectedIndex = -1;
Invalidate();
@@ -254,6 +258,7 @@
fPackages.insert(itInsertionPt, package);
Invalidate(_RectOfIndex(insertionIndex)
| _RectOfIndex(fPackages.size() - 1));
+ package->AddListener(fPackageListener);
}
}
@@ -266,6 +271,7 @@
fSelectedIndex = -1;
if (fSelectedIndex > index)
fSelectedIndex--;
+ fPackages[index]->RemoveListener(fPackageListener);
fPackages.erase(fPackages.begin() + index);
if (fPackages.empty())
Invalidate();
@@ -302,9 +308,12 @@
int32 _IndexOfPackage(PackageInfoRef package) const
{
- if (package.Get() != NULL)
- return _IndexOfName(package->Name());
- return -1;
+ std::vector<PackageInfoRef>::const_iterator it
+ = std::lower_bound(fPackages.begin(), fPackages.end(),
package,
+ &_IsPackageBefore);
+
+ return (it == fPackages.end() || (*it)->Name() !=
package->Name())
+ ? -1 : it - fPackages.begin();
}
diff --git a/src/apps/haikudepot/ui/MainWindow.cpp
b/src/apps/haikudepot/ui/MainWindow.cpp
index d9e4890..dbc1877 100644
--- a/src/apps/haikudepot/ui/MainWindow.cpp
+++ b/src/apps/haikudepot/ui/MainWindow.cpp
@@ -205,8 +205,6 @@
_UpdateAuthorization();
_RestoreWindowFrame(settings);
- atomic_set(&fPackagesToShowListID, 0);
-
// start worker threads
BPackageRoster().StartWatching(this,
B_WATCH_PACKAGE_INSTALLATION_LOCATIONS);
@@ -271,15 +269,6 @@
if (fPendingActionsWorker >= 0)
wait_for_thread(fPendingActionsWorker, NULL);
- delete_sem(fPackageToPopulateSem);
- if (fPopulatePackageWorker >= 0)
- wait_for_thread(fPopulatePackageWorker, NULL);
-
- delete_sem(fNewPackagesToShowSem);
- delete_sem(fShowPackagesAcknowledgeSem);
- if (fShowPackagesWorker >= 0)
- wait_for_thread(fShowPackagesWorker, NULL);
-
if (fScreenshotWindow != NULL && fScreenshotWindow->Lock())
fScreenshotWindow->Quit();
}
@@ -436,17 +425,14 @@
{
BString name;
if (message->FindString("name", &name) == B_OK) {
- BAutolock locker(fModel.Lock());
- int count = fVisiblePackages.CountItems();
- for (int i = 0; i < count; i++) {
- const PackageInfoRef& package
- =
fVisiblePackages.ItemAtFast(i);
- if (package.Get() != NULL &&
package->Name() == name) {
- locker.Unlock();
- _AdoptPackage(package);
- break;
- }
+ PackageInfoRef package;
+ {
+ BAutolock locker(fModel.Lock());
+ package = fModel.PackageForName(name);
}
+ if (package.Get() == NULL)
+ debugger("unable to find the named
package");
+ _AdoptPackage(package);
} else {
_ClearPackage();
}
@@ -506,43 +492,7 @@
BAutolock locker(fModel.Lock());
fModel.SetPackageState(ref,
ref->State());
}
-
- // Asynchronous updates to the package
information
- // can mean that the package needs to be added
or
- // removed to/from the visible packages when
the current
- // filter parameters are re-evaluated on this
package.
- bool wasVisible =
fVisiblePackages.Contains(ref);
- bool isVisible;
- {
- BAutolock locker(fModel.Lock());
- // The package didn't get a chance yet
to be in the
- // visible package list
- isVisible = fModel.MatchesFilter(ref);
-
- // Transfer this single package,
otherwise we miss
- // other packages if they appear or
disappear along
- // with this one before receive a
notification for
- // them.
- if (isVisible) {
- fVisiblePackages.Add(ref);
- } else if (wasVisible)
- fVisiblePackages.Remove(ref);
- }
-
- if (wasVisible != isVisible) {
- if (!isVisible) {
- if (fPackageListView != NULL)
-
fPackageListView->RemovePackage(ref);
- if (fFeaturedPackagesView !=
NULL)
-
fFeaturedPackagesView->RemovePackage(ref);
- } else {
- if (fPackageListView != NULL)
-
fPackageListView->AddPackage(ref);
- if (fFeaturedPackagesView !=
NULL && ref->IsProminent())
-
fFeaturedPackagesView->AddPackage(ref);
- }
- }
-
+ _AddRemovePackageFromLists(ref);
if (!fSinglePackageMode && (changes &
PKG_CHANGED_STATE) != 0
&& fCoordinator == NULL) {
fWorkStatusView->PackageStatusChanged(ref);
@@ -575,69 +525,6 @@
fWorkStatusView->SetIdle();
break;
- case MSG_ADD_VISIBLE_PACKAGES:
- {
- struct SemaphoreReleaser {
- SemaphoreReleaser(sem_id semaphore)
- :
- fSemaphore(semaphore)
- { }
-
- ~SemaphoreReleaser() { release_sem(fSemaphore);
}
-
- sem_id fSemaphore;
- };
-
- // Make sure acknowledge semaphore is released even on
error,
- // so the worker thread won't be blocked
- SemaphoreReleaser
acknowledger(fShowPackagesAcknowledgeSem);
-
- int32 numPackages = 0;
- type_code unused;
- status_t status = message->GetInfo("package_ref",
&unused,
- &numPackages);
- if (status != B_OK || numPackages == 0)
- break;
-
- int32 listID = 0;
- status = message->FindInt32("list_id", &listID);
- if (status != B_OK)
- break;
- if (listID != atomic_get(&fPackagesToShowListID)) {
- // list is outdated, ignore
- break;
- }
-
- for (int i = 0; i < numPackages; i++) {
- PackageInfo* packageRaw = NULL;
- status = message->FindPointer("package_ref", i,
- (void**)&packageRaw);
- if (status != B_OK)
- break;
- PackageInfoRef package(packageRaw, true);
-
- if (fPackageListView != NULL)
- fPackageListView->AddPackage(package);
- if (fFeaturedPackagesView != NULL &&
package->IsProminent())
-
fFeaturedPackagesView->AddPackage(package);
- }
- break;
- }
-
- case MSG_UPDATE_SELECTED_PACKAGE:
- {
- const PackageInfoRef& selectedPackage =
fPackageInfoView->Package();
- if (fFeaturedPackagesView != NULL)
-
fFeaturedPackagesView->SelectPackage(selectedPackage, true);
- if (fPackageListView != NULL)
-
fPackageListView->SelectPackage(selectedPackage);
-
- AutoLocker<BLocker> modelLocker(fModel.Lock());
- if
(!fVisiblePackages.Contains(fPackageInfoView->Package()))
- fPackageInfoView->Clear();
- break;
- }
-
case MSG_USER_USAGE_CONDITIONS_NOT_LATEST:
{
BMessage userDetailMsg;
@@ -909,43 +796,15 @@
resume_thread(fPopulatePackageWorker);
} else
fPopulatePackageWorker = -1;
-
- fNewPackagesToShowSem = create_sem(0, "ShowPackages");
- fShowPackagesAcknowledgeSem = create_sem(0, "ShowPackagesAck");
- if (fNewPackagesToShowSem >= 0 && fShowPackagesAcknowledgeSem >= 0) {
- fShowPackagesWorker = spawn_thread(&_PackagesToShowWorker,
- "Good news everyone", B_NORMAL_PRIORITY, this);
- if (fShowPackagesWorker >= 0)
- resume_thread(fShowPackagesWorker);
- } else
- fShowPackagesWorker = -1;
}
void
-MainWindow::_AdoptModel()
+MainWindow::_AdoptModelControls()
{
- if (Logger::IsTraceEnabled())
- printf("adopting model to main window ui");
-
if (fSinglePackageMode)
return;
- {
- AutoLocker<BLocker> modelLocker(fModel.Lock());
- fVisiblePackages = fModel.CreatePackageList();
- AutoLocker<BLocker> listLocker(fPackagesToShowListLock);
- fPackagesToShowList = fVisiblePackages;
- atomic_add(&fPackagesToShowListID, 1);
- }
-
- if (fFeaturedPackagesView != NULL)
- fFeaturedPackagesView->Clear();
- if (fPackageListView != NULL)
- fPackageListView->Clear();
-
- release_sem(fNewPackagesToShowSem);
-
BAutolock locker(fModel.Lock());
fShowAvailablePackagesItem->SetMarked(fModel.ShowAvailablePackages());
fShowInstalledPackagesItem->SetMarked(fModel.ShowInstalledPackages());
@@ -962,6 +821,51 @@
void
+MainWindow::_AdoptModel()
+{
+ if (Logger::IsTraceEnabled())
+ printf("adopting model to main window ui");
+
+ if (fSinglePackageMode)
+ return;
+
+ fModel.Lock()->Lock();
+ const DepotList& depots = fModel.Depots();
+ fModel.Lock()->Unlock();
+
+ for (int32 d = 0; d < depots.CountItems(); d++) {
+ const DepotInfo& depot = depots.ItemAtFast(d);
+ const PackageList& packages = depot.Packages();
+ for (int32 p = 0; p < packages.CountItems(); p++)
+ _AddRemovePackageFromLists(packages.ItemAtFast(p));
+ }
+
+ _AdoptModelControls();
+}
+
+
+void
+MainWindow::_AddRemovePackageFromLists(const PackageInfoRef& package)
+{
+ bool matches;
+
+ {
+ AutoLocker<BLocker> modelLocker(fModel.Lock());
+ matches = fModel.MatchesFilter(package);
+ }
+
+ if (matches) {
+ if (package->IsProminent())
+ fFeaturedPackagesView->AddPackage(package);
+ fPackageListView->AddPackage(package);
+ } else {
+ fFeaturedPackagesView->RemovePackage(package);
+ fPackageListView->RemovePackage(package);
+ }
+}
+
+
+void
MainWindow::_AdoptPackage(const PackageInfoRef& package)
{
{
@@ -988,6 +892,12 @@
void
MainWindow::_StartBulkLoad(bool force)
{
+ if (fFeaturedPackagesView != NULL)
+ fFeaturedPackagesView->Clear();
+ if (fPackageListView != NULL)
+ fPackageListView->Clear();
+ fPackageInfoView->Clear();
+
fRefreshRepositoriesItem->SetEnabled(false);
ProcessCoordinator* bulkLoadCoordinator =
ProcessCoordinatorFactory::CreateBulkLoadCoordinator(
@@ -1176,78 +1086,6 @@
}
-/* static */ status_t
-MainWindow::_PackagesToShowWorker(void* arg)
-{
- MainWindow* window = reinterpret_cast<MainWindow*>(arg);
-
- while (acquire_sem(window->fNewPackagesToShowSem) == B_OK) {
- PackageList packageList;
- int32 listID = 0;
- {
- AutoLocker<BLocker>
lock(&window->fPackagesToShowListLock);
- packageList = window->fPackagesToShowList;
- listID = atomic_get(&window->fPackagesToShowListID);
- window->fPackagesToShowList.Clear();
- }
-
- // Add packages to list views in batches of kPackagesPerUpdate
so we
- // don't block the window thread for long with each iteration
- enum {
- kPackagesPerUpdate = 20
- };
- uint32 packagesInMessage = 0;
- BMessage message(MSG_ADD_VISIBLE_PACKAGES);
- BMessenger messenger(window);
- bool listIsOutdated = false;
-
- for (int i = 0; i < packageList.CountItems(); i++) {
- const PackageInfoRef& package =
packageList.ItemAtFast(i);
-
- if (packagesInMessage >= kPackagesPerUpdate) {
- if (listID !=
atomic_get(&window->fPackagesToShowListID)) {
- // The model was changed again in the
meantime, and thus
- // our package list isn't current
anymore. Send no further
- // messags based on this list and go
back to start.
- listIsOutdated = true;
- break;
- }
-
- message.AddInt32("list_id", listID);
- messenger.SendMessage(&message);
- message.MakeEmpty();
- packagesInMessage = 0;
-
- // Don't spam the window's message queue, which
would make it
- // unresponsive (i.e. allows UI messages to get
in between our
- // messages). When it has processed the message
we just sent,
- // it will let us know by releasing the
semaphore.
-
acquire_sem(window->fShowPackagesAcknowledgeSem);
- }
- package->AcquireReference();
- message.AddPointer("package_ref", package.Get());
- packagesInMessage++;
- }
-
- if (listIsOutdated)
- continue;
-
- // Send remaining package infos, if any, which didn't make it
into
- // the last message (count < kPackagesPerUpdate)
- if (packagesInMessage > 0) {
- message.AddInt32("list_id", listID);
- messenger.SendMessage(&message);
- acquire_sem(window->fShowPackagesAcknowledgeSem);
- }
-
- // Update selected package in list views
- messenger.SendMessage(MSG_UPDATE_SELECTED_PACKAGE);
- }
-
- return 0;
-}
-
-
void
MainWindow::_OpenLoginWindow(const BMessage& onSuccessMessage)
{
diff --git a/src/apps/haikudepot/ui/MainWindow.h
b/src/apps/haikudepot/ui/MainWindow.h
index 0dddeae..32a3717 100644
--- a/src/apps/haikudepot/ui/MainWindow.h
+++ b/src/apps/haikudepot/ui/MainWindow.h
@@ -87,7 +87,10 @@
void
_RestoreModelSettings(const BMessage& settings);
void _InitWorkerThreads();
+ void _AdoptModelControls();
void _AdoptModel();
+ void
_AddRemovePackageFromLists(
+ const
PackageInfoRef& package);
void _AdoptPackage(const
PackageInfoRef& package);
void _ClearPackage();
@@ -151,7 +154,6 @@
Model fModel;
ModelListenerRef fModelListener;
- PackageList fVisiblePackages;
std::queue<ProcessCoordinator*>
fCoordinatorQueue;
@@ -171,14 +173,6 @@
bool fForcePopulatePackage;
BLocker fPackageToPopulateLock;
sem_id fPackageToPopulateSem;
-
- thread_id fShowPackagesWorker;
- PackageList fPackagesToShowList;
- int32 fPackagesToShowListID;
- // atomic, counted up whenever
fPackagesToShowList is refilled
- BLocker fPackagesToShowListLock;
- sem_id fNewPackagesToShowSem;
- sem_id
fShowPackagesAcknowledgeSem;
};
#endif // MAIN_WINDOW_H
--
To view, visit https://review.haiku-os.org/c/haiku/+/2748
To unsubscribe, or for help writing mail filters, visit
https://review.haiku-os.org/settings
Gerrit-Project: haiku
Gerrit-Branch: master
Gerrit-Change-Id: I393cee929695726539602b51630ae285fb8384f1
Gerrit-Change-Number: 2748
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Lindesay <apl@xxxxxxxxxxxxxx>
Gerrit-MessageType: newchange
