diff --git a/gpt4all-chat/qml/ModelSettings.qml b/gpt4all-chat/qml/ModelSettings.qml index 2435e08f..4cbc5b56 100644 --- a/gpt4all-chat/qml/ModelSettings.qml +++ b/gpt4all-chat/qml/ModelSettings.qml @@ -92,7 +92,7 @@ MySettingsTab { enabled: root.currentModelInfo.isClone text: qsTr("Remove") onClicked: { - ModelList.removeClone(root.currentModelInfo); + ModelList.uninstall(root.currentModelInfo); comboBox.currentIndex = 0; } } diff --git a/gpt4all-chat/src/download.cpp b/gpt4all-chat/src/download.cpp index 62ff73fb..7d183768 100644 --- a/gpt4all-chat/src/download.cpp +++ b/gpt4all-chat/src/download.cpp @@ -330,36 +330,27 @@ void Download::installCompatibleModel(const QString &modelName, const QString &a void Download::removeModel(const QString &modelFile) { - const QString filePath = MySettings::globalInstance()->modelPath() + modelFile; - QFile incompleteFile(ModelList::globalInstance()->incompleteDownloadPath(modelFile)); - if (incompleteFile.exists()) { + auto *modelList = ModelList::globalInstance(); + auto *mySettings = MySettings::globalInstance(); + + QFile incompleteFile(modelList->incompleteDownloadPath(modelFile)); + if (incompleteFile.exists()) incompleteFile.remove(); - } bool shouldRemoveInstalled = false; - QFile file(filePath); + + QFile file(mySettings->modelPath() + modelFile); + const ModelInfo info = modelList->modelInfoByFilename(modelFile); + + Network::globalInstance()->trackEvent("remove_model", { {"model", modelFile}, {"exists", file.exists()} }); + if (file.exists()) { - const ModelInfo info = ModelList::globalInstance()->modelInfoByFilename(modelFile); - MySettings::globalInstance()->eraseModel(info.id()); - shouldRemoveInstalled = info.installed && !info.isClone() && (info.isDiscovered() || info.isCompatibleApi || info.description() == "" /*indicates sideloaded*/); - if (shouldRemoveInstalled) - ModelList::globalInstance()->removeInstalled(info); - Network::globalInstance()->trackEvent("remove_model", { {"model", modelFile} }); - file.remove(); - emit toastMessage(tr("Model \"%1\" is removed.").arg(info.name())); + modelList->uninstall(info); + if (!info.isClone()) + file.remove(); } - if (!shouldRemoveInstalled) { - QVector> data { - { ModelList::InstalledRole, false }, - { ModelList::BytesReceivedRole, 0 }, - { ModelList::BytesTotalRole, 0 }, - { ModelList::TimestampRole, 0 }, - { ModelList::SpeedRole, QString() }, - { ModelList::DownloadErrorRole, QString() }, - }; - ModelList::globalInstance()->updateDataByFilename(modelFile, data); - } + emit toastMessage(tr("Model \"%1\" is removed.").arg(info.name())); } void Download::handleSslErrors(QNetworkReply *reply, const QList &errors) diff --git a/gpt4all-chat/src/modellist.cpp b/gpt4all-chat/src/modellist.cpp index 17538349..4867ef63 100644 --- a/gpt4all-chat/src/modellist.cpp +++ b/gpt4all-chat/src/modellist.cpp @@ -1041,21 +1041,21 @@ void ModelList::updateDataByFilename(const QString &filename, QVector modelsById; { QMutexLocker locker(&m_mutex); + QStringList modelsById; for (auto *info : std::as_const(m_models)) if (info->filename() == filename) modelsById.append(info->id()); - } - if (modelsById.isEmpty()) { - qWarning() << "ERROR: cannot update model as list does not contain file" << filename; - return; - } + if (modelsById.isEmpty()) { + qWarning() << "ERROR: cannot update model as list does not contain file" << filename; + return; + } - for (auto &id : std::as_const(modelsById)) - updateData(id, data); + for (auto &id : std::as_const(modelsById)) + updateDataInternal(id, data, locker, /*relock*/ &id != &modelsById.constLast()); + } } ModelInfo ModelList::modelInfo(const QString &id) const @@ -1118,53 +1118,84 @@ QString ModelList::clone(const ModelInfo &model) return id; } -void ModelList::removeClone(const ModelInfo &model) +void ModelList::uninstall(const ModelInfo &model) { Q_ASSERT(QThread::currentThread() == thread()); - Q_ASSERT(model.isClone()); - if (!model.isClone()) - return; - removeInternal(model); - emit layoutChanged(); + { + QMutexLocker lock(&m_mutex); + + if (!model.isClone()) { + QStringList modelsById; + auto filename = model.filename(); + for (const auto *info : std::as_const(m_models)) + if (info->filename() == filename && info->isClone()) + modelsById << info->id(); + + for (const auto &id : std::as_const(modelsById)) + removeInternal(id, lock); + } + + auto id = model.id(); + if (model.isClone() || model.isDiscovered() || model.isCompatibleApi + || model.description() == "" /*sideloaded*/ + ) { + // model can be completely removed from list + removeInternal(id, lock, /*relock*/ false); + emit selectableModelListChanged(); + } else { + // Model came from models.json and needs to be reset instead of removed + QVector> data { + { ModelList::InstalledRole, false }, + { ModelList::BytesReceivedRole, 0 }, + { ModelList::BytesTotalRole, 0 }, + { ModelList::TimestampRole, 0 }, + { ModelList::SpeedRole, QString() }, + { ModelList::DownloadErrorRole, QString() }, + }; + updateDataInternal(id, data, lock, /*relock*/ false); + + // erase settings + MySettings::globalInstance()->eraseModel(id); + } + } } -void ModelList::removeInstalled(const ModelInfo &model) +void ModelList::removeInternal(const QString &id, QMutexLocker &lock, bool relock) { Q_ASSERT(QThread::currentThread() == thread()); - Q_ASSERT(model.installed); - Q_ASSERT(!model.isClone()); - Q_ASSERT(model.isDiscovered() || model.isCompatibleApi || model.description() == "" /*indicates sideloaded*/); - removeInternal(model); - emit layoutChanged(); -} + Q_ASSERT(lock.isLocked()); -void ModelList::removeInternal(const ModelInfo &model) -{ - Q_ASSERT(QThread::currentThread() == thread()); - const bool hasModel = contains(model.id()); + bool hasModel = m_modelMap.contains(id); Q_ASSERT(hasModel); if (!hasModel) { - qWarning() << "ERROR: model list does not contain" << model.id(); + qWarning() << "ERROR: model list does not contain" << id; return; } - int indexOfModel = 0; - { - QMutexLocker locker(&m_mutex); - ModelInfo *info = m_modelMap.value(model.id()); - indexOfModel = m_models.indexOf(info); - } - beginRemoveRows(QModelIndex(), indexOfModel, indexOfModel); - { - QMutexLocker locker(&m_mutex); - ModelInfo *info = m_models.takeAt(indexOfModel); - m_modelMap.remove(info->id()); - delete info; - } + auto mapIt = std::as_const(m_modelMap).find(id); + qsizetype listIdx = m_models.indexOf(*mapIt); + + lock.unlock(); + beginRemoveRows({}, listIdx, listIdx); + lock.relock(); + + // remove entry + auto *info = *mapIt; + Q_ASSERT(std::as_const(m_modelMap).find(id) == mapIt); + Q_ASSERT(m_models.indexOf(info) == listIdx); + m_modelMap.erase(mapIt); + m_models.remove(listIdx); + delete info; + + lock.unlock(); endRemoveRows(); - emit selectableModelListChanged(); - MySettings::globalInstance()->eraseModel(model.id()); + + // erase settings + MySettings::globalInstance()->eraseModel(id); + + if (relock) + lock.relock(); } QString ModelList::uniqueModelName(const ModelInfo &model) const @@ -1927,15 +1958,14 @@ void ModelList::setDiscoverSort(DiscoverSort sort) void ModelList::clearDiscoveredModels() { // NOTE: This could be made much more efficient - QList infos; - { - QMutexLocker locker(&m_mutex); - for (auto *info : std::as_const(m_models)) - if (info->isDiscovered() && !info->installed) - infos.append(*info); - } - for (auto &info : std::as_const(infos)) - removeInternal(info); + QMutexLocker locker(&m_mutex); + QStringList ids; + for (auto *info : std::as_const(m_models)) + if (info->isDiscovered() && !info->installed) + ids << info->id(); + + for (auto &id : std::as_const(ids)) + removeInternal(id, locker, /*relock*/ &id != &ids.constLast()); } float ModelList::discoverProgress() const diff --git a/gpt4all-chat/src/modellist.h b/gpt4all-chat/src/modellist.h index 3431f8c5..9292d31b 100644 --- a/gpt4all-chat/src/modellist.h +++ b/gpt4all-chat/src/modellist.h @@ -421,8 +421,8 @@ public: Q_INVOKABLE ModelInfo modelInfoByFilename(const QString &filename) const; Q_INVOKABLE bool isUniqueName(const QString &name) const; Q_INVOKABLE QString clone(const ModelInfo &model); - Q_INVOKABLE void removeClone(const ModelInfo &model); - Q_INVOKABLE void removeInstalled(const ModelInfo &model); + // delist a model and erase its settings, or mark it as not installed if built-in + void uninstall(const ModelInfo &model); ModelInfo defaultModelInfo() const; void addModel(const QString &id); @@ -501,7 +501,9 @@ private: void updateDataInternal(const QString &id, const QVector> &data, QMutexLocker &lock, bool relock = true); - void removeInternal(const ModelInfo &model); + // call with lock held + void removeInternal(const QString &id, QMutexLocker &lock, bool relock = true); + void clearDiscoveredModels(); bool modelExists(const QString &fileName) const; int indexForModel(ModelInfo *model);