From 3b686b36aadf39523dc7520f5fe8e4061026e289 Mon Sep 17 00:00:00 2001 From: Jan-Erik Rediger Date: Wed, 21 Jan 2026 15:28:34 +0100 Subject: [PATCH] Serialize values using MessagePack instead of bincode The bincode crate isn't maintained anymore. While it's been stable and without issues for us for years, switching to anotherformat is easy while we're switching the database anyway. MessagePack can be even smaller than bincode for the same data (just a couple of bytes here and there). Whether it's actually faster has not been benchmarked. Compared to everything else the (de)serialization overhead is probably a small fraction of the whole thing. Why do we need serialization anyway? Ping assembly does not have any knowledge of metrics. It only knows what's in the database. So in order to put in in the right place in the ping payload we need to know the type of the stored data. That data needs to be somewhere. By serializing the whole value (the `Metric` enum) we can deserialize it into that enum and the serde part takes care of "knowing" the type. --- Cargo.lock | 24 +++++++++++++++++++++-- glean-core/Cargo.toml | 1 + glean-core/benchmark/Cargo.lock | 20 +++++++++++++++++++ glean-core/src/database/sqlite.rs | 8 ++++---- supply-chain/imports.lock | 32 +++++++++++++++++++++++-------- 5 files changed, 71 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 77171672cc..eddf0ceea9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -618,6 +618,7 @@ dependencies = [ "pulldown-cmark", "pulldown-cmark-to-cmark", "rkv", + "rmp-serde", "rusqlite", "serde", "serde_json", @@ -920,9 +921,9 @@ dependencies = [ [[package]] name = "num-traits" -version = "0.2.15" +version = "0.2.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "578ede34cf02f8924ab9447f50c28075b4d3e5b269972345e7e0372b38c6cdcd" +checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841" dependencies = [ "autocfg", ] @@ -1166,6 +1167,25 @@ dependencies = [ "wr_malloc_size_of", ] +[[package]] +name = "rmp" +version = "0.8.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4ba8be72d372b2c9b35542551678538b562e7cf86c3315773cae48dfbfe7790c" +dependencies = [ + "num-traits", +] + +[[package]] +name = "rmp-serde" +version = "1.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72f81bee8c8ef9b577d1681a70ebbc962c232461e397b22c208c43c04b67a155" +dependencies = [ + "rmp", + "serde", +] + [[package]] name = "rusqlite" version = "0.37.0" diff --git a/glean-core/Cargo.toml b/glean-core/Cargo.toml index ba154d34c1..71f5bdd897 100644 --- a/glean-core/Cargo.toml +++ b/glean-core/Cargo.toml @@ -45,6 +45,7 @@ env_logger = { version = "0.10.0", default-features = false, optional = true } malloc_size_of_derive = "0.1.3" malloc_size_of = { version = "0.2.2", package = "wr_malloc_size_of", default-features = false, features = ["once_cell"] } rusqlite = { version = "0.37.0", features = ["bundled"] } +rmp-serde = "1.3.1" [target.'cfg(target_os = "android")'.dependencies] android_logger = { version = "0.12.0", default-features = false } diff --git a/glean-core/benchmark/Cargo.lock b/glean-core/benchmark/Cargo.lock index ccdf5cd27b..a11de53917 100644 --- a/glean-core/benchmark/Cargo.lock +++ b/glean-core/benchmark/Cargo.lock @@ -561,6 +561,7 @@ dependencies = [ "once_cell", "oslog", "rkv", + "rmp-serde", "rusqlite", "serde", "serde_json", @@ -1185,6 +1186,25 @@ dependencies = [ "wr_malloc_size_of", ] +[[package]] +name = "rmp" +version = "0.8.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4ba8be72d372b2c9b35542551678538b562e7cf86c3315773cae48dfbfe7790c" +dependencies = [ + "num-traits", +] + +[[package]] +name = "rmp-serde" +version = "1.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72f81bee8c8ef9b577d1681a70ebbc962c232461e397b22c208c43c04b67a155" +dependencies = [ + "rmp", + "serde", +] + [[package]] name = "rusqlite" version = "0.37.0" diff --git a/glean-core/src/database/sqlite.rs b/glean-core/src/database/sqlite.rs index a0022d77ae..242e92b124 100644 --- a/glean-core/src/database/sqlite.rs +++ b/glean-core/src/database/sqlite.rs @@ -117,7 +117,7 @@ impl Database { |row| { let id: String = row.get(0)?; let blob: Vec = row.get(1)?; - let blob: Metric = bincode::deserialize(&blob) + let blob: Metric = rmp_serde::from_slice(&blob) .map_err(|_| FromSqlError::InvalidType)?; Ok((id, blob)) }, @@ -237,7 +237,7 @@ impl Database { "#; let mut stmt = tx.prepare_cached(insert_sql)?; - let encoded = bincode::serialize(&metric).expect("IMPOSSIBLE: Serializing metric failed"); + let encoded = rmp_serde::to_vec(&metric).expect("IMPOSSIBLE: Serializing metric failed"); stmt.execute(params![key, storage_name, lifetime.as_str(), encoded])?; Ok(()) @@ -317,7 +317,7 @@ impl Database { if let Ok(Some(row)) = rows.next() { let blob: Vec = row.get(0)?; - let old_value = bincode::deserialize(&blob).ok(); + let old_value = rmp_serde::from_slice(&blob).ok(); transform(old_value) } else { transform(None) @@ -337,7 +337,7 @@ impl Database { { let mut stmt = tx.prepare_cached(insert_sql)?; let encoded = - bincode::serialize(&new_value).expect("IMPOSSIBLE: Serializing metric failed"); + rmp_serde::to_vec(&new_value).expect("IMPOSSIBLE: Serializing metric failed"); stmt.execute(params![key, storage_name, lifetime.as_str(), encoded])?; } diff --git a/supply-chain/imports.lock b/supply-chain/imports.lock index 6d0c66dc6c..b2c71badea 100644 --- a/supply-chain/imports.lock +++ b/supply-chain/imports.lock @@ -660,6 +660,12 @@ criteria = "safe-to-deploy" delta = "0.7.1 -> 0.8.0" notes = "Minor updates, using new Rust features like `const`, no major changes." +[[audits.bytecode-alliance.audits.num-traits]] +who = "Andrew Brown " +criteria = "safe-to-deploy" +version = "0.2.19" +notes = "As advertised: a numeric library. The only `unsafe` is from some float-to-int conversions, which seems expected." + [[audits.bytecode-alliance.audits.percent-encoding]] who = "Alex Crichton " criteria = "safe-to-deploy" @@ -2064,7 +2070,7 @@ who = "Jan-Erik Rediger " criteria = "safe-to-deploy" user-id = 111105 # Mark Hammond (mhammond) start = "2025-03-18" -end = "2026-03-25" +end = "2027-04-23" aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" [[audits.mozilla.audits.android-tzdata]] @@ -2409,13 +2415,6 @@ but convinced myself that any generated code will be entirely safe to deploy. """ aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" -[[audits.mozilla.audits.num-traits]] -who = "Josh Stone " -criteria = "safe-to-deploy" -version = "0.2.15" -notes = "All code written or reviewed by Josh Stone." -aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" - [[audits.mozilla.audits.once_cell]] who = "Erich Gubler " criteria = "safe-to-deploy" @@ -2472,6 +2471,23 @@ criteria = "safe-to-deploy" version = "0.18.4" aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" +[[audits.mozilla.audits.rmp]] +who = "Ben Dean-Kawamura " +criteria = "safe-to-deploy" +version = "0.8.14" +notes = """ +Very popular crate. 1 instance of unsafe code, which is used to adjust a slice to work around +lifetime issues. No network or file access. +""" +aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" + +[[audits.mozilla.audits.rmp-serde]] +who = "Ben Dean-Kawamura " +criteria = "safe-to-deploy" +version = "1.3.0" +notes = "Very popular crate. No unsafe code, network or file access." +aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" + [[audits.mozilla.audits.rusqlite]] who = "Mike Hommey " criteria = "safe-to-deploy"