perf: optimize mod toggling (#791)

This commit is contained in:
Julian Chang 2026-09-05 18:03:04 +02:00 • committed by LynithDev
parent 0217a54518
commit 7780a8900c
11 changed files with 394 additions and 167 deletions

View file

@ -1,74 +0,0 @@
{
"db_name": "SQLite",
"query": "\n\t\tSELECT cluster_id, hash, cluster_file_name, enabled, seen_status\n\t\tFROM cluster_artifacts\n\t\tWHERE cluster_id = ?\n\t\t",
"describe": {
"columns": [
{
"name": "cluster_id",
"ordinal": 0,
"type_info": "Integer",
"origin": {
"Table": {
"table": "cluster_artifacts",
"name": "cluster_id"
}
}
},
{
"name": "hash",
"ordinal": 1,
"type_info": "Text",
"origin": {
"Table": {
"table": "cluster_artifacts",
"name": "hash"
}
}
},
{
"name": "cluster_file_name",
"ordinal": 2,
"type_info": "Text",
"origin": {
"Table": {
"table": "cluster_artifacts",
"name": "cluster_file_name"
}
}
},
{
"name": "enabled",
"ordinal": 3,
"type_info": "Integer",
"origin": {
"Table": {
"table": "cluster_artifacts",
"name": "enabled"
}
}
},
{
"name": "seen_status",
"ordinal": 4,
"type_info": "Integer",
"origin": {
"Table": {
"table": "cluster_artifacts",
"name": "seen_status"
}
}
}
],
"parameters": {
"Right": 1
},
"nullable": [
false,
false,
false,
false,
false
]
},
"hash": "055b35798b158025888aba14bab7c8f5111c1786202f2399f6383d290abfe9cc"
}

View file

@ -0,0 +1,158 @@
{
"db_name": "SQLite",
"query": "\n\t\tSELECT\n\t\t\tca.hash AS \"hash!: String\",\n\t\t\tca.cluster_file_name AS \"cluster_file_name!: String\",\n\t\t\tca.enabled AS \"enabled!: i64\",\n\t\t\tca.seen_status AS \"seen_status!: i64\",\n\t\t\ta.content_type AS \"content_type!: i64\",\n\t\t\ta.file_name AS \"file_name!: String\",\n\t\t\tpr.provider AS \"provider?: i64\",\n\t\t\tpr.project_id AS \"project_id?: String\",\n\t\t\tpr.version_id AS \"version_id?: String\",\n\t\t\tpr.display_name AS \"display_name?: String\",\n\t\t\tpr.display_version AS \"display_version?: String\",\n\t\t\tpr.published_at AS \"published_at?: String\"\n\t\tFROM cluster_artifacts ca\n\t\tJOIN artifacts a ON a.hash = ca.hash\n\t\tLEFT JOIN provider_releases pr ON pr.rowid = (\n\t\t\tSELECT rowid\n\t\t\tFROM provider_releases\n\t\t\tWHERE hash = ca.hash\n\t\t\tORDER BY published_at DESC, provider, project_id, version_id\n\t\t\tLIMIT 1\n\t\t)\n\t\tWHERE ca.cluster_id = ?\n\t\t",
"describe": {
"columns": [
{
"name": "hash!: String",
"ordinal": 0,
"type_info": "Text",
"origin": {
"Table": {
"table": "cluster_artifacts",
"name": "hash"
}
}
},
{
"name": "cluster_file_name!: String",
"ordinal": 1,
"type_info": "Text",
"origin": {
"Table": {
"table": "cluster_artifacts",
"name": "cluster_file_name"
}
}
},
{
"name": "enabled!: i64",
"ordinal": 2,
"type_info": "Integer",
"origin": {
"Table": {
"table": "cluster_artifacts",
"name": "enabled"
}
}
},
{
"name": "seen_status!: i64",
"ordinal": 3,
"type_info": "Integer",
"origin": {
"Table": {
"table": "cluster_artifacts",
"name": "seen_status"
}
}
},
{
"name": "content_type!: i64",
"ordinal": 4,
"type_info": "Integer",
"origin": {
"Table": {
"table": "artifacts",
"name": "content_type"
}
}
},
{
"name": "file_name!: String",
"ordinal": 5,
"type_info": "Text",
"origin": {
"Table": {
"table": "artifacts",
"name": "file_name"
}
}
},
{
"name": "provider?: i64",
"ordinal": 6,
"type_info": "Integer",
"origin": {
"Table": {
"table": "provider_releases",
"name": "provider"
}
}
},
{
"name": "project_id?: String",
"ordinal": 7,
"type_info": "Text",
"origin": {
"Table": {
"table": "provider_releases",
"name": "project_id"
}
}
},
{
"name": "version_id?: String",
"ordinal": 8,
"type_info": "Text",
"origin": {
"Table": {
"table": "provider_releases",
"name": "version_id"
}
}
},
{
"name": "display_name?: String",
"ordinal": 9,
"type_info": "Text",
"origin": {
"Table": {
"table": "provider_releases",
"name": "display_name"
}
}
},
{
"name": "display_version?: String",
"ordinal": 10,
"type_info": "Text",
"origin": {
"Table": {
"table": "provider_releases",
"name": "display_version"
}
}
},
{
"name": "published_at?: String",
"ordinal": 11,
"type_info": "Text",
"origin": {
"Table": {
"table": "provider_releases",
"name": "published_at"
}
}
}
],
"parameters": {
"Right": 1
},
"nullable": [
false,
false,
false,
false,
false,
false,
true,
true,
true,
true,
true,
true
]
},
"hash": "d1fc4c93f507d459e590cda5b5830a1e26e3047139db08a9f6b90c84185402a6"
}

View file

@ -135,9 +135,10 @@ impl Component for PackageRow {
let manifest_default = item.manifest_default;
(move |()| {
if let Some(h) = &hash {
cluster.mutate(ClusterAction::ToggleArtifact {
cluster.mutate(ClusterAction::SetArtifactEnabled {
cluster_id,
hash: h.clone(),
enabled: !enabled_now,
});
} else if let Some(bundle) = &bundle_name {
cluster.mutate(ClusterAction::SetBundlePackageEnabled {

View file

@ -42,6 +42,17 @@ pub async fn invalidate_cluster_content_queries() {
QueriesStorage::<ClusterContentQuery>::invalidate_all().await;
}
/// Everything [`invalidate_cluster_queries`] does bar the cluster list and the
/// cached package updates neither of which an enabled flag can move
/// The bundle queries do move a toggle writes a bundle override and both read
/// those back
async fn invalidate_enabled_flag_queries() {
timed("cluster_content", QueriesStorage::<ClusterContentQuery>::invalidate_all()).await;
timed("bundle_overrides", QueriesStorage::<BundleOverridesQuery>::invalidate_all()).await;
timed("bundles_with_status", QueriesStorage::<BundlesWithStatusQuery>::invalidate_all()).await;
timed("bundle_updates", QueriesStorage::<BundleUpdatesQuery>::invalidate_all()).await;
}
pub async fn invalidate_profile_queries() {
QueriesStorage::<ListNamedProfilesQuery>::invalidate_all().await;
QueriesStorage::<GameProfileQuery>::invalidate_all().await;
@ -55,9 +66,10 @@ pub struct ClusterMutation;
#[derive(Clone, PartialEq, Eq, Hash)]
pub enum ClusterAction {
ToggleArtifact {
SetArtifactEnabled {
cluster_id: ClusterId,
hash: String,
enabled: bool,
},
RemoveArtifact {
cluster_id: ClusterId,
@ -96,19 +108,21 @@ impl MutationCapability for ClusterMutation {
let services = &state.services;
let content = &state.services.content();
let result = match keys {
ClusterAction::ToggleArtifact { cluster_id, hash } => {
oneclient_core::toggle_artifact_enabled(*cluster_id, hash, content)
.await
.map(|(_, live)| {
if live == LiveSync::Deferred && state.games.is_active(*cluster_id) {
services
.events
.notify("Saved for the next launch")
.body("Minecraft is running, but this could not be added to the open game.")
.send();
}
})
}
ClusterAction::SetArtifactEnabled {
cluster_id,
hash,
enabled,
} => oneclient_core::set_artifact_enabled_to(*cluster_id, hash, *enabled, content)
.await
.map(|live| {
if live == LiveSync::Deferred && state.games.is_active(*cluster_id) {
services
.events
.notify("Saved for the next launch")
.body("Minecraft is running, but this could not be added to the open game.")
.send();
}
}),
ClusterAction::RemoveArtifact { cluster_id, hash } => {
oneclient_core::remove_artifact_from_cluster(*cluster_id, hash, true, content).await
}
@ -177,13 +191,17 @@ impl MutationCapability for ClusterMutation {
result.map_err(|e| e.to_string())
}
async fn on_settled(&self, _keys: &ClusterAction, result: &Result<(), String>) {
async fn on_settled(&self, keys: &ClusterAction, result: &Result<(), String>) {
if let Err(err) = result
&& let Ok(state) = crate::launcher::state()
{
state.services.events.notify("Action failed").body(err).error().send();
}
invalidate_cluster_queries().await;
if matches!(keys, ClusterAction::SetArtifactEnabled { .. }) {
invalidate_enabled_flag_queries().await;
} else {
invalidate_cluster_queries().await;
}
}
}

View file

@ -451,7 +451,7 @@ pub async fn set_bundle_package_override(
}
/// For bundle files the cluster has not installed
/// installed ones go through [`toggle_artifact_enabled`]
/// installed ones go through [`set_artifact_enabled_to`]
/// Matching the manifest default clears the override
/// switching *on* also drops objections filed under other bundles
#[tracing::instrument(level = "debug", skip(ctx))]
@ -562,26 +562,7 @@ pub async fn on_user_remove_artifact(
handle_user_artifact_action(cluster_id, hash, ctx, OverrideType::Removed).await
}
#[tracing::instrument(level = "debug", skip(ctx))]
#[tracing::instrument(level = "debug", skip(ctx))]
pub async fn toggle_artifact_enabled(
cluster_id: i64,
hash: &str,
ctx: &ContentCtx,
) -> ContentResult<(bool, LiveSync)> {
let (enabled, live) = PackageStore::set_artifact_enabled(cluster_id, hash, ctx).await?;
if enabled {
on_user_enable_artifact(cluster_id, hash, ctx).await?;
} else {
on_user_disable_artifact(cluster_id, hash, ctx).await?;
}
Ok((enabled, live))
}
/// Prefer this over [`toggle_artifact_enabled`] unless the current value is
/// known to be the opposite
/// The caller passes the value it wants
/// a relinked artifact keeps its old `enabled` so a flip on an already-correct
/// row puts it wrong
#[tracing::instrument(level = "debug", skip(ctx))]
@ -590,14 +571,16 @@ pub async fn set_artifact_enabled_to(
hash: &str,
enabled: bool,
ctx: &ContentCtx,
) -> ContentResult<()> {
PackageStore::set_artifact_enabled_to(cluster_id, hash, enabled, ctx).await?;
) -> ContentResult<LiveSync> {
let (_, live) = PackageStore::set_artifact_enabled_to(cluster_id, hash, enabled, ctx).await?;
if enabled {
on_user_enable_artifact(cluster_id, hash, ctx).await
on_user_enable_artifact(cluster_id, hash, ctx).await?;
} else {
on_user_disable_artifact(cluster_id, hash, ctx).await
on_user_disable_artifact(cluster_id, hash, ctx).await?;
}
Ok(live)
}
/// Writes the override alongside the flag
@ -680,10 +663,7 @@ pub async fn remove_artifact_from_cluster(
.await?
.and_then(|artifact| ContentType::from_repr(artifact.content_type as u8));
let link = artifact_dao::list_cluster_artifacts(&ctx.db, cluster_id)
.await?
.into_iter()
.find(|l| l.hash == hash);
let link = artifact_dao::get_cluster_artifact(&ctx.db, cluster_id, hash).await?;
// Database first unconditionally
// on Windows a jar held open by a running game blocks deleting every hard

View file

@ -19,7 +19,6 @@ pub use install::{
reconcile_duplicate_activity,
remove_artifact_from_cluster, set_artifact_enabled_to, set_bundle_package_enabled,
set_bundle_package_opt_in,
toggle_artifact_enabled,
set_bundle_package_override, set_bundle_package_overrides,
};
pub use manager::{Bundle, BundlesManager};

View file

@ -189,40 +189,26 @@ impl PackageStore {
cluster_id: i64,
ctx: &ContentCtx,
) -> ContentResult<Vec<LinkedArtifactInfo>> {
let links = artifact_dao::list_cluster_artifacts(&ctx.db, cluster_id).await?;
let mut items = Vec::with_capacity(links.len());
let rows = artifact_dao::list_cluster_artifacts_detailed(&ctx.db, cluster_id).await?;
for link in links {
let Some(artifact) =
artifact_dao::get_artifact_by_hash(&ctx.db, &link.hash).await?
else {
continue;
};
let content_type = ContentType::from_repr(artifact.content_type as u8)
.unwrap_or(ContentType::Mod);
let release = artifact_dao::get_release_by_hash(&ctx.db, &link.hash).await?;
let seen_status = link.status();
items.push(LinkedArtifactInfo {
hash: link.hash,
cluster_file_name: link.cluster_file_name,
enabled: link.enabled != 0,
content_type,
file_name: artifact.file_name,
project_id: release.as_ref().map(|r| r.project_id.clone()),
version_id: release.as_ref().map(|r| r.version_id.clone()),
display_name: release.as_ref().map(|r| r.display_name.clone()),
display_version: release.as_ref().map(|r| r.display_version.clone()),
provider: release
.as_ref()
.and_then(|r| ProviderId::from_repr(r.provider as u8)),
published_at: release.as_ref().and_then(|r| r.published_at.clone()),
seen_status,
});
}
Ok(items)
Ok(rows
.into_iter()
.map(|row| LinkedArtifactInfo {
hash: row.hash,
cluster_file_name: row.cluster_file_name,
enabled: row.enabled != 0,
content_type: ContentType::from_repr(row.content_type as u8)
.unwrap_or(ContentType::Mod),
file_name: row.file_name,
project_id: row.project_id,
version_id: row.version_id,
display_name: row.display_name,
display_version: row.display_version,
provider: row.provider.and_then(|p| ProviderId::from_repr(p as u8)),
published_at: row.published_at,
seen_status: SeenStatus::from_repr(row.seen_status).unwrap_or_default(),
})
.collect())
}
#[tracing::instrument(level = "debug", skip(ctx))]
@ -256,10 +242,8 @@ impl PackageStore {
hash: &str,
enabled: bool,
ctx: &ContentCtx,
) -> ContentResult<bool> {
Self::write_artifact_enabled(cluster_id, hash, Some(enabled), ctx)
.await
.map(|(enabled, _)| enabled)
) -> ContentResult<(bool, LiveSync)> {
Self::write_artifact_enabled(cluster_id, hash, Some(enabled), ctx).await
}
/// `target` of `None` means "the opposite of whatever it is now"

View file

@ -34,7 +34,7 @@ pub use oneclient_content::bundles::{
ApplyBundleUpdatesResult, Bundle, BundleArchive, BundleError, BundleFile,
BundleFileKind, BundleManifest, BundleOptionalPackage, BundlesManager, BundleUpdateCheckResult,
BundleWithUpdateStatus, FileUpdateStatus, get_bundles_with_update_status,
remove_artifact_from_cluster, toggle_artifact_enabled,
remove_artifact_from_cluster, set_artifact_enabled_to,
};
pub use changelog::{fetch_changelog, parse_changelog, ChangelogGroup};
pub use tos::{fetch_terms, TermsDocument};

View file

@ -1,6 +1,8 @@
use sqlx::SqlitePool;
use crate::models::{ArtifactRow, ClusterArtifactRow, ProviderReleaseRow, SeenStatus};
use crate::models::{
ArtifactRow, ClusterArtifactRow, LinkedArtifactRow, ProviderReleaseRow, SeenStatus,
};
pub async fn get_artifact_by_hash(
pool: &SqlitePool,
@ -263,16 +265,39 @@ pub async fn unlink_cluster_artifact(
Ok(())
}
pub async fn list_cluster_artifacts(
/// One artifact can carry several `provider_releases` rows so the release
/// columns are taken from a single row picked by a total order rather than
/// grouped which lets SQLite mix columns from different rows
pub async fn list_cluster_artifacts_detailed(
pool: &SqlitePool,
cluster_id: i64,
) -> Result<Vec<ClusterArtifactRow>, sqlx::Error> {
) -> Result<Vec<LinkedArtifactRow>, sqlx::Error> {
sqlx::query_as!(
ClusterArtifactRow,
LinkedArtifactRow,
r#"
SELECT cluster_id, hash, cluster_file_name, enabled, seen_status
FROM cluster_artifacts
WHERE cluster_id = ?
SELECT
ca.hash AS "hash!: String",
ca.cluster_file_name AS "cluster_file_name!: String",
ca.enabled AS "enabled!: i64",
ca.seen_status AS "seen_status!: i64",
a.content_type AS "content_type!: i64",
a.file_name AS "file_name!: String",
pr.provider AS "provider?: i64",
pr.project_id AS "project_id?: String",
pr.version_id AS "version_id?: String",
pr.display_name AS "display_name?: String",
pr.display_version AS "display_version?: String",
pr.published_at AS "published_at?: String"
FROM cluster_artifacts ca
JOIN artifacts a ON a.hash = ca.hash
LEFT JOIN provider_releases pr ON pr.rowid = (
SELECT rowid
FROM provider_releases
WHERE hash = ca.hash
ORDER BY published_at DESC, provider, project_id, version_id
LIMIT 1
)
WHERE ca.cluster_id = ?
"#,
cluster_id
)
@ -364,3 +389,121 @@ pub async fn mark_all_seen(pool: &SqlitePool) -> Result<u64, sqlx::Error> {
Ok(result.rows_affected())
}
#[cfg(test)]
mod tests {
use super::*;
use crate::dao::cluster as cluster_dao;
use crate::models::NewCluster;
async fn pool() -> SqlitePool {
let pool = SqlitePool::connect("sqlite::memory:")
.await
.expect("in-memory sqlite");
sqlx::migrate!().run(&pool).await.expect("migrations run");
pool
}
async fn seed_cluster(pool: &SqlitePool) -> i64 {
cluster_dao::insert(
pool,
&NewCluster {
name: "26.1 fabric",
folder_name: "26.1 fabric",
mc_version: "26.1",
mc_loader: 1,
mc_loader_version: None,
setting_profile_name: None,
stage: 0,
},
)
.await
.expect("insert cluster")
.id
}
#[tokio::test]
async fn detailed_listing_matches_the_lookups_it_replaced() {
let pool = pool().await;
let cluster_id = seed_cluster(&pool).await;
insert_artifact(&pool, "hash_managed", 0, "a/managed.jar", "managed.jar", Some(1))
.await
.expect("insert managed artifact");
insert_artifact(&pool, "hash_local", 0, "a/local.jar", "local.jar", None)
.await
.expect("insert local artifact");
for version in ["v1", "v2"] {
upsert_provider_release(
&pool,
1,
"sodium",
version,
"hash_managed",
"Sodium",
version,
None,
"[]",
"[]",
)
.await
.expect("insert release");
}
link_cluster_artifact(&pool, cluster_id, "hash_managed", "managed.jar")
.await
.expect("link managed");
link_cluster_artifact(&pool, cluster_id, "hash_local", "local.jar")
.await
.expect("link local");
update_cluster_artifact(&pool, cluster_id, "hash_local", "local.jar", 0)
.await
.expect("disable local");
let rows = list_cluster_artifacts_detailed(&pool, cluster_id)
.await
.expect("detailed listing");
assert_eq!(rows.len(), 2, "one row per link, not per provider release");
let managed = rows.iter().find(|r| r.hash == "hash_managed").expect("managed row");
assert_eq!(managed.file_name, "managed.jar");
assert_eq!(managed.enabled, 1);
assert_eq!(managed.project_id.as_deref(), Some("sodium"));
assert_eq!(managed.provider, Some(1));
assert_eq!(managed.version_id.as_deref(), Some("v1"));
assert_eq!(
managed.display_version.as_deref(),
Some("v1"),
"every release column comes from the same release row"
);
let local = rows.iter().find(|r| r.hash == "hash_local").expect("local row");
assert_eq!(local.enabled, 0);
assert!(local.project_id.is_none(), "a local file has no provider columns");
let mut conn = pool.acquire().await.expect("connection");
sqlx::query("PRAGMA foreign_keys = OFF")
.execute(&mut *conn)
.await
.expect("disable foreign keys");
sqlx::query!("DELETE FROM artifacts WHERE hash = ?", "hash_local")
.execute(&mut *conn)
.await
.expect("drop artifact row");
let orphans: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM cluster_artifacts WHERE hash = ?")
.bind("hash_local")
.fetch_one(&mut *conn)
.await
.expect("count orphan link");
assert_eq!(orphans, 1, "the link must outlive its artifact row");
drop(conn);
let rows = list_cluster_artifacts_detailed(&pool, cluster_id)
.await
.expect("detailed listing after delete");
assert_eq!(rows.len(), 1);
assert_eq!(rows[0].hash, "hash_managed");
}
}

View file

@ -61,3 +61,19 @@ impl ClusterArtifactRow {
SeenStatus::from_repr(self.seen_status).unwrap_or_default()
}
}
#[derive(Debug, Clone, FromRow)]
pub struct LinkedArtifactRow {
pub hash: String,
pub cluster_file_name: String,
pub enabled: i64,
pub seen_status: i64,
pub content_type: i64,
pub file_name: String,
pub provider: Option<i64>,
pub project_id: Option<String>,
pub version_id: Option<String>,
pub display_name: Option<String>,
pub display_version: Option<String>,
pub published_at: Option<String>,
}

View file

@ -9,7 +9,9 @@ mod java;
mod package_metadata;
mod setting_profile;
pub use artifact::{ArtifactRow, ClusterArtifactRow, ProviderReleaseRow, SeenStatus};
pub use artifact::{
ArtifactRow, ClusterArtifactRow, LinkedArtifactRow, ProviderReleaseRow, SeenStatus,
};
pub use browser_package_update::BrowserPackageUpdateRow;
pub use package_metadata::PackageMetadataRow;
pub use bundle::{BundleRow, NewBundle};