hrev54245 adds 1 changeset to branch 'master'
old head: 9d8d114499db1f50f105c48c1a2172ea56e18bcd
new head: c419919252f9076761900fd4e7575407a554255e
overview:
https://git.haiku-os.org/haiku/log/?qt=range&q=c419919252f9+%5E9d8d114499db
----------------------------------------------------------------------------
c419919252f9: 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
Change-Id: I393cee929695726539602b51630ae285fb8384f1
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2748
Reviewed-by: Adrien Destugues <pulkomandy@xxxxxxxxx>
[ Andrew Lindesay <apl@xxxxxxxxxxxxxx> ]
----------------------------------------------------------------------------
Revision: hrev54245
Commit: c419919252f9076761900fd4e7575407a554255e
URL: https://git.haiku-os.org/haiku/commit/?id=c419919252f9
Author: Andrew Lindesay <apl@xxxxxxxxxxxxxx>
Date: Tue May 19 10:53:10 2020 UTC
Ticket: https://dev.haiku-os.org/ticket/16012
----------------------------------------------------------------------------
8 files changed, 92 insertions(+), 263 deletions(-)
src/apps/haikudepot/HaikuDepotConstants.h | 2 -
src/apps/haikudepot/model/Model.cpp | 33 +--
src/apps/haikudepot/model/Model.h | 4 +-
.../server/ServerIconExportUpdateProcess.cpp | 2 +-
.../server/ServerPkgDataUpdateProcess.cpp | 4 +-
src/apps/haikudepot/ui/FeaturedPackagesView.cpp | 15 +-
src/apps/haikudepot/ui/MainWindow.cpp | 283 ++++---------------
src/apps/haikudepot/ui/MainWindow.h | 12 +-
----------------------------------------------------------------------------
diff --git a/src/apps/haikudepot/HaikuDepotConstants.h
b/src/apps/haikudepot/HaikuDepotConstants.h
index 4b5be6139f..bb72497766 100644
--- a/src/apps/haikudepot/HaikuDepotConstants.h
+++ b/src/apps/haikudepot/HaikuDepotConstants.h
@@ -11,8 +11,6 @@ enum {
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 e2352ebc76..de427d082a 100644
--- a/src/apps/haikudepot/model/Model.cpp
+++ b/src/apps/haikudepot/model/Model.cpp
@@ -353,29 +353,19 @@ Model::AddListener(const ModelListenerRef& listener)
}
-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 @@ Model::MatchesFilter(const PackageInfoRef& package) const
{
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 7fd161cce2..1aa57cca83 100644
--- a/src/apps/haikudepot/model/Model.h
+++ b/src/apps/haikudepot/model/Model.h
@@ -77,9 +77,7 @@ public:
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 a8d14bae05..d1d59d1e6a 100644
--- a/src/apps/haikudepot/server/ServerIconExportUpdateProcess.cpp
+++ b/src/apps/haikudepot/server/ServerIconExportUpdateProcess.cpp
@@ -147,7 +147,7 @@ ServerIconExportUpdateProcess::PopulateForDepot(const
DepotInfo& depot)
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 6e4695a998..fc6cc9c11c 100644
--- a/src/apps/haikudepot/server/ServerPkgDataUpdateProcess.cpp
+++ b/src/apps/haikudepot/server/ServerPkgDataUpdateProcess.cpp
@@ -200,11 +200,11 @@ PackageFillingPkgListener::Handle(DumpExportPkg* pkg)
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 10427ccc54..1dd9b0f042 100644
--- a/src/apps/haikudepot/ui/FeaturedPackagesView.cpp
+++ b/src/apps/haikudepot/ui/FeaturedPackagesView.cpp
@@ -199,6 +199,10 @@ public:
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 @@ public:
fPackages.insert(itInsertionPt, package);
Invalidate(_RectOfIndex(insertionIndex)
| _RectOfIndex(fPackages.size() - 1));
+ package->AddListener(fPackageListener);
}
}
@@ -266,6 +271,7 @@ public:
fSelectedIndex = -1;
if (fSelectedIndex > index)
fSelectedIndex--;
+ fPackages[index]->RemoveListener(fPackageListener);
fPackages.erase(fPackages.begin() + index);
if (fPackages.empty())
Invalidate();
@@ -302,9 +308,12 @@ public:
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 d9e4890e06..28764a14df 100644
--- a/src/apps/haikudepot/ui/MainWindow.cpp
+++ b/src/apps/haikudepot/ui/MainWindow.cpp
@@ -205,8 +205,6 @@ MainWindow::MainWindow(const BMessage& settings)
_UpdateAuthorization();
_RestoreWindowFrame(settings);
- atomic_set(&fPackagesToShowListID, 0);
-
// start worker threads
BPackageRoster().StartWatching(this,
B_WATCH_PACKAGE_INSTALLATION_LOCATIONS);
@@ -271,15 +269,6 @@ MainWindow::~MainWindow()
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,15 @@ MainWindow::MessageReceived(BMessage* message)
{
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");
+ else
+ _AdoptPackage(package);
} else {
_ClearPackage();
}
@@ -506,43 +493,7 @@ MainWindow::MessageReceived(BMessage* message)
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 +526,6 @@ MainWindow::MessageReceived(BMessage* message)
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 +797,15 @@ MainWindow::_InitWorkerThreads()
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());
@@ -961,6 +821,51 @@ MainWindow::_AdoptModel()
}
+void
+MainWindow::_AdoptModel()
+{
+ if (Logger::IsTraceEnabled())
+ printf("adopting model to main window ui\n");
+
+ 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 +893,12 @@ MainWindow::_ClearPackage()
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 +1087,6 @@ MainWindow::_PopulatePackageWorker(void* arg)
}
-/* 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 0dddeae636..32a3717d9c 100644
--- a/src/apps/haikudepot/ui/MainWindow.h
+++ b/src/apps/haikudepot/ui/MainWindow.h
@@ -87,7 +87,10 @@ private:
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 @@ private:
Model fModel;
ModelListenerRef fModelListener;
- PackageList fVisiblePackages;
std::queue<ProcessCoordinator*>
fCoordinatorQueue;
@@ -171,14 +173,6 @@ private:
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