diff --git a/crates/filemeta/benches/xl_meta_bench.rs b/crates/filemeta/benches/xl_meta_bench.rs index ad1f8e6c4..ee46865a4 100644 --- a/crates/filemeta/benches/xl_meta_bench.rs +++ b/crates/filemeta/benches/xl_meta_bench.rs @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -use criterion::{Criterion, criterion_group, criterion_main}; -use rustfs_filemeta::{FileMeta, test_data::*}; +use criterion::{Criterion, Throughput, criterion_group, criterion_main}; +use rustfs_filemeta::{FileMeta, MetaCacheEntry, test_data::*}; use std::hint::black_box; fn bench_create_real_xlmeta(c: &mut Criterion) { @@ -81,6 +81,20 @@ fn bench_version_stats(c: &mut Criterion) { c.bench_function("version_stats", |b| b.iter(|| black_box(fm.get_version_stats()))); } +fn bench_into_fileinfo_realistic(c: &mut Criterion) { + let data = create_realistic_object_xlmeta().unwrap(); + let fm = FileMeta::load(&data).unwrap(); + + c.bench_function("into_fileinfo_realistic", |b| { + b.iter(|| { + black_box( + fm.into_fileinfo(black_box("bucket"), black_box("object"), black_box(""), false, false, true) + .unwrap(), + ) + }) + }); +} + fn bench_validate_integrity(c: &mut Criterion) { let data = create_real_xlmeta().unwrap(); let fm = FileMeta::load(&data).unwrap(); @@ -93,6 +107,33 @@ fn bench_validate_integrity(c: &mut Criterion) { }); } +// Simulates the per-object CPU cost of a LIST page: each entry carries raw xl.meta bytes +// (cached = None), so to_fileinfo runs the full FileMeta::load + into_fileinfo path that a +// listing performs for every returned object. Reports objects/sec via Throughput::Elements. +fn bench_list_page_to_fileinfo(c: &mut Criterion) { + const N: u64 = 10_000; + let data = create_realistic_object_xlmeta().unwrap(); + let entries: Vec = (0..N) + .map(|i| MetaCacheEntry { + name: format!("prefix/object-{i:08}"), + metadata: data.clone(), + cached: None, + reusable: false, + }) + .collect(); + + let mut group = c.benchmark_group("list_page_to_fileinfo"); + group.throughput(Throughput::Elements(N)); + group.bench_function("realistic_10k", |b| { + b.iter(|| { + for entry in &entries { + black_box(entry.to_fileinfo(black_box("bucket")).unwrap()); + } + }) + }); + group.finish(); +} + criterion_group!( benches, bench_create_real_xlmeta, @@ -104,7 +145,9 @@ criterion_group!( bench_round_trip_real_xlmeta, bench_round_trip_complex_xlmeta, bench_version_stats, - bench_validate_integrity + bench_validate_integrity, + bench_into_fileinfo_realistic, + bench_list_page_to_fileinfo ); criterion_main!(benches); diff --git a/crates/filemeta/src/filemeta/version.rs b/crates/filemeta/src/filemeta/version.rs index 208ef3875..2cc46b886 100644 --- a/crates/filemeta/src/filemeta/version.rs +++ b/crates/filemeta/src/filemeta/version.rs @@ -2044,13 +2044,11 @@ impl MetaObject { } for (k, v) in &self.meta_sys { - let lower_k = k.to_lowercase(); - - if has_internal_suffix(&lower_k, SUFFIX_TIER_FV_ID) || has_internal_suffix(&lower_k, SUFFIX_TIER_FV_MARKER) { + if has_internal_suffix(k, SUFFIX_TIER_FV_ID) || has_internal_suffix(k, SUFFIX_TIER_FV_MARKER) { continue; } - if lower_k == AMZ_STORAGE_CLASS.to_lowercase() && v == b"STANDARD" { + if k.eq_ignore_ascii_case(AMZ_STORAGE_CLASS) && v == b"STANDARD" { continue; } diff --git a/crates/filemeta/src/test_data.rs b/crates/filemeta/src/test_data.rs index 157595b08..e5cb73982 100644 --- a/crates/filemeta/src/test_data.rs +++ b/crates/filemeta/src/test_data.rs @@ -357,6 +357,70 @@ pub fn create_complex_xlmeta() -> Result> { fm.marshal_msg() } +/// Create a realistic single-object xl.meta with a populated `meta_sys` (internal keys) and +/// `meta_user` (user metadata), mirroring what a typical PUT produces. Unlike the other +/// fixtures (which leave `meta_sys` empty), this exercises the internal-key classification and +/// lookup hot path in `into_fileinfo`. +pub fn create_realistic_object_xlmeta() -> Result> { + use rustfs_utils::http::{ + SUFFIX_ACTUAL_SIZE, SUFFIX_COMPRESSION, SUFFIX_COMPRESSION_SIZE, SUFFIX_CRC, SUFFIX_INLINE_DATA, insert_bytes, + }; + + let mut fm = FileMeta::new(); + + let version_id = Uuid::parse_str("01234567-89ab-cdef-0123-456789abcdef")?; + let data_dir = Uuid::parse_str("fedcba98-7654-3210-fedc-ba9876543210")?; + + let mut meta_user = HashMap::new(); + meta_user.insert("content-type".to_string(), "application/octet-stream".to_string()); + meta_user.insert("X-Amz-Meta-Author".to_string(), "test-user".to_string()); + meta_user.insert("X-Amz-Meta-Project".to_string(), "rustfs".to_string()); + meta_user.insert("X-Amz-Meta-Env".to_string(), "production".to_string()); + + // Dual-writes x-rustfs-internal-* and x-minio-internal-* keys, matching real objects. + let mut meta_sys = HashMap::new(); + insert_bytes(&mut meta_sys, SUFFIX_COMPRESSION, b"klauspost/compress/s2".to_vec()); + insert_bytes(&mut meta_sys, SUFFIX_COMPRESSION_SIZE, b"1048576".to_vec()); + insert_bytes(&mut meta_sys, SUFFIX_ACTUAL_SIZE, b"2097152".to_vec()); + insert_bytes(&mut meta_sys, SUFFIX_CRC, b"crc32c:abcdef01".to_vec()); + insert_bytes(&mut meta_sys, SUFFIX_INLINE_DATA, b"true".to_vec()); + + let object_version = MetaObject { + version_id: Some(version_id), + data_dir: Some(data_dir), + erasure_algorithm: crate::fileinfo::ErasureAlgo::ReedSolomon, + erasure_m: 4, + erasure_n: 2, + erasure_block_size: 1024 * 1024, + erasure_index: 1, + erasure_dist: vec![0, 1, 2, 3, 4, 5], + bitrot_checksum_algo: ChecksumAlgo::HighwayHash, + part_numbers: vec![1], + part_etags: vec!["d41d8cd98f00b204e9800998ecf8427e".to_string()], + part_sizes: vec![2097152], + part_actual_sizes: vec![2097152], + part_indices: Vec::new(), + size: 2097152, + mod_time: Some(OffsetDateTime::from_unix_timestamp(1705312200)?), + meta_sys, + meta_user, + }; + + let file_version = FileMetaVersion { + version_type: VersionType::Object, + legacy_object: None, + object: Some(object_version), + delete_marker: None, + write_version: 1, + uses_legacy_checksum: false, + }; + + let shallow_version = FileMetaShallowVersion::try_from(file_version)?; + fm.versions.push(shallow_version); + + fm.marshal_msg() +} + /// Create a corrupted xl.meta file for error handling tests pub fn create_corrupted_xlmeta() -> Vec { let mut data = vec![ diff --git a/crates/utils/src/http/metadata_compat.rs b/crates/utils/src/http/metadata_compat.rs index 16b7625a8..ba4334411 100644 --- a/crates/utils/src/http/metadata_compat.rs +++ b/crates/utils/src/http/metadata_compat.rs @@ -54,29 +54,55 @@ pub const SUFFIX_TIER_FV_ID: &str = "tier-free-versionID"; pub const SUFFIX_TIER_FV_MARKER: &str = "tier-free-marker"; pub const SUFFIX_TIER_SKIP_FV_ID: &str = "tier-skip-fvid"; +/// Case-insensitive (ASCII) check that `s` begins with `prefix`. Equivalent to +/// `s.to_lowercase().starts_with(prefix)` when `prefix` is ASCII (as both internal prefixes are), +/// but without allocating. +fn starts_with_ignore_ascii_case(s: &str, prefix: &str) -> bool { + s.len() >= prefix.len() && s.as_bytes()[..prefix.len()].eq_ignore_ascii_case(prefix.as_bytes()) +} + +/// Allocation-free equivalent of `key.to_lowercase() == format!("{prefix}{suffix}")`. +/// The common ASCII path matches the prefix case-insensitively and ASCII-lowercases the suffix +/// region byte-for-byte. Non-ASCII keys fall back to full Unicode lowercasing so the result is +/// identical to the original for every input (e.g. U+212A KELVIN SIGN lowercases to ASCII `k`). +fn internal_key_eq(key: &str, prefix: &str, suffix: &str) -> bool { + if !key.is_ascii() { + return key.to_lowercase() == format!("{prefix}{suffix}"); + } + let key = key.as_bytes(); + if key.len() != prefix.len() + suffix.len() { + return false; + } + let (key_prefix, key_suffix) = key.split_at(prefix.len()); + key_prefix.eq_ignore_ascii_case(prefix.as_bytes()) + && key_suffix + .iter() + .zip(suffix.as_bytes()) + .all(|(k, s)| k.to_ascii_lowercase() == *s) +} + /// Returns true if the key is an internal metadata key (x-rustfs-internal-* or x-minio-internal-*) /// for xl.meta compatibility. Case-insensitive. pub fn is_internal_key(key: &str) -> bool { - let lower = key.to_lowercase(); - lower.starts_with(RUSTFS_INTERNAL_PREFIX) || lower.starts_with(MINIO_INTERNAL_PREFIX) + starts_with_ignore_ascii_case(key, RUSTFS_INTERNAL_PREFIX) || starts_with_ignore_ascii_case(key, MINIO_INTERNAL_PREFIX) } /// Returns true if the key matches the given suffix for either x-rustfs-internal-* or x-minio-internal-*. pub fn has_internal_suffix(key: &str, suffix: &str) -> bool { - let lower = key.to_lowercase(); - let rustfs_key = format!("{RUSTFS_INTERNAL_PREFIX}{suffix}"); - let minio_key = format!("{MINIO_INTERNAL_PREFIX}{suffix}"); - lower == rustfs_key || lower == minio_key + internal_key_eq(key, RUSTFS_INTERNAL_PREFIX, suffix) || internal_key_eq(key, MINIO_INTERNAL_PREFIX, suffix) } /// Strips x-rustfs-internal- or x-minio-internal- prefix from key. Returns the suffix part. /// Case-insensitive. Returns None if key is not an internal key. pub fn strip_internal_prefix(key: &str) -> Option { - let lower = key.to_lowercase(); - lower - .strip_prefix(RUSTFS_INTERNAL_PREFIX) - .or_else(|| lower.strip_prefix(MINIO_INTERNAL_PREFIX)) - .map(|s| s.to_string()) + let rest = if starts_with_ignore_ascii_case(key, RUSTFS_INTERNAL_PREFIX) { + &key[RUSTFS_INTERNAL_PREFIX.len()..] + } else if starts_with_ignore_ascii_case(key, MINIO_INTERNAL_PREFIX) { + &key[MINIO_INTERNAL_PREFIX.len()..] + } else { + return None; + }; + Some(rest.to_lowercase()) } /// Returns true if key is internal and its suffix part starts with the given suffix_prefix. @@ -96,6 +122,29 @@ fn both_keys(suffix: &str) -> (String, String) { (format!("{RUSTFS_INTERNAL_PREFIX}{suffix}"), format!("{MINIO_INTERNAL_PREFIX}{suffix}")) } +/// Longest known suffix ("objectlock-retention-timestamp", 30 bytes) plus the 18-byte prefix fits +/// well under this; the cap leaves ample headroom so lookups never touch the heap in practice. +const INTERNAL_KEY_STACK_CAP: usize = 96; + +/// Builds the internal key `{prefix}{suffix}` in a stack buffer and invokes `f` with it, avoiding +/// the per-lookup heap allocation that `format!` incurs. Falls back to an owned `String` only for +/// unusually long suffixes that do not fit the stack buffer. +fn with_internal_key(prefix: &str, suffix: &str, f: impl FnOnce(&str) -> R) -> R { + let total = prefix.len() + suffix.len(); + if total <= INTERNAL_KEY_STACK_CAP { + let mut buf = [0u8; INTERNAL_KEY_STACK_CAP]; + buf[..prefix.len()].copy_from_slice(prefix.as_bytes()); + buf[prefix.len()..total].copy_from_slice(suffix.as_bytes()); + // `prefix` and `suffix` are both `&str`, so their concatenation is valid UTF-8. + match std::str::from_utf8(&buf[..total]) { + Ok(key) => f(key), + Err(_) => f(&format!("{prefix}{suffix}")), + } + } else { + f(&format!("{prefix}{suffix}")) + } +} + /// Builds the RustFS internal key for the given suffix. Use when a single key is needed (e.g. for /// backward compat). Prefer insert_str/get_str when both keys should be written/read. pub fn internal_key_rustfs(suffix: &str) -> String { @@ -111,27 +160,35 @@ pub fn insert_str(map: &mut HashMap, suffix: &str, value: String } pub fn get_str(map: &HashMap, suffix: &str) -> Option { + if let Some(v) = with_internal_key(RUSTFS_INTERNAL_PREFIX, suffix, |k1| map.get(k1).cloned()) { + return Some(v); + } + if let Some(v) = with_internal_key(MINIO_INTERNAL_PREFIX, suffix, |k2| map.get(k2).cloned()) { + return Some(v); + } + // Rare fallback: case-insensitive scan for non-canonical key casing. let (k1, k2) = both_keys(suffix); - map.get(&k1).cloned().or_else(|| map.get(&k2).cloned()).or_else(|| { - map.iter() - .find(|(key, _)| key.eq_ignore_ascii_case(&k1) || key.eq_ignore_ascii_case(&k2)) - .map(|(_, value)| value.clone()) - }) + map.iter() + .find(|(key, _)| key.eq_ignore_ascii_case(&k1) || key.eq_ignore_ascii_case(&k2)) + .map(|(_, value)| value.clone()) } pub fn contains_key_str(map: &HashMap, suffix: &str) -> bool { + if with_internal_key(RUSTFS_INTERNAL_PREFIX, suffix, |k1| map.contains_key(k1)) { + return true; + } + if with_internal_key(MINIO_INTERNAL_PREFIX, suffix, |k2| map.contains_key(k2)) { + return true; + } let (k1, k2) = both_keys(suffix); - map.contains_key(&k1) - || map.contains_key(&k2) - || map - .keys() - .any(|key| key.eq_ignore_ascii_case(&k1) || key.eq_ignore_ascii_case(&k2)) + map.keys() + .any(|key| key.eq_ignore_ascii_case(&k1) || key.eq_ignore_ascii_case(&k2)) } pub fn remove_str(map: &mut HashMap, suffix: &str) { + with_internal_key(RUSTFS_INTERNAL_PREFIX, suffix, |k1| map.remove(k1)); + with_internal_key(MINIO_INTERNAL_PREFIX, suffix, |k2| map.remove(k2)); let (k1, k2) = both_keys(suffix); - map.remove(&k1); - map.remove(&k2); map.retain(|key, _| !key.eq_ignore_ascii_case(&k1) && !key.eq_ignore_ascii_case(&k2)); } @@ -145,19 +202,18 @@ pub fn insert_bytes(map: &mut HashMap>, suffix: &str, value: Vec } pub fn get_bytes(map: &HashMap>, suffix: &str) -> Option> { - let (k1, k2) = both_keys(suffix); - map.get(&k1).cloned().or_else(|| map.get(&k2).cloned()) + with_internal_key(RUSTFS_INTERNAL_PREFIX, suffix, |k1| map.get(k1).cloned()) + .or_else(|| with_internal_key(MINIO_INTERNAL_PREFIX, suffix, |k2| map.get(k2).cloned())) } pub fn contains_key_bytes(map: &HashMap>, suffix: &str) -> bool { - let (k1, k2) = both_keys(suffix); - map.contains_key(&k1) || map.contains_key(&k2) + with_internal_key(RUSTFS_INTERNAL_PREFIX, suffix, |k1| map.contains_key(k1)) + || with_internal_key(MINIO_INTERNAL_PREFIX, suffix, |k2| map.contains_key(k2)) } pub fn remove_bytes(map: &mut HashMap>, suffix: &str) { - let (k1, k2) = both_keys(suffix); - map.remove(&k1); - map.remove(&k2); + with_internal_key(RUSTFS_INTERNAL_PREFIX, suffix, |k1| map.remove(k1)); + with_internal_key(MINIO_INTERNAL_PREFIX, suffix, |k2| map.remove(k2)); } #[cfg(test)] @@ -239,4 +295,119 @@ mod tests { assert_eq!(get_bytes(&meta_sys, SUFFIX_TRANSITIONED_VERSION_ID), Some(b"rustfs-version".to_vec())); } + + // Reference implementations mirroring the original allocation-heavy logic, used to prove the + // optimized helpers are behavior-preserving across a battery of inputs. + fn is_internal_key_ref(key: &str) -> bool { + let lower = key.to_lowercase(); + lower.starts_with(RUSTFS_INTERNAL_PREFIX) || lower.starts_with(MINIO_INTERNAL_PREFIX) + } + + fn has_internal_suffix_ref(key: &str, suffix: &str) -> bool { + let lower = key.to_lowercase(); + lower == format!("{RUSTFS_INTERNAL_PREFIX}{suffix}") || lower == format!("{MINIO_INTERNAL_PREFIX}{suffix}") + } + + fn strip_internal_prefix_ref(key: &str) -> Option { + let lower = key.to_lowercase(); + lower + .strip_prefix(RUSTFS_INTERNAL_PREFIX) + .or_else(|| lower.strip_prefix(MINIO_INTERNAL_PREFIX)) + .map(|s| s.to_string()) + } + + #[test] + fn test_classifiers_match_reference_impl() { + let keys = [ + "x-rustfs-internal-inline-data", + "X-RustFS-Internal-Inline-Data", + "x-minio-internal-compression", + "X-MINIO-INTERNAL-actual-size", + "x-rustfs-internal-", + "x-rustfs-internal-replication-reset-arn:aws:s3:::bucket", + "x-rustfs-internal-Actual-Object-Size", + "x-amz-meta-custom", + "content-type", + "not-internal", + "", + "X", + "x-rustfs-interna", // one char short of the prefix + "x-rustfs-internal-tier-free-mar\u{212A}er", // U+212A KELVIN SIGN lowercases to ASCII 'k' + ]; + let suffixes = [ + SUFFIX_INLINE_DATA, + SUFFIX_COMPRESSION, + SUFFIX_ACTUAL_SIZE, + SUFFIX_ACTUAL_OBJECT_SIZE_CAP, + SUFFIX_PURGESTATUS, + SUFFIX_TIER_FV_MARKER, + "inline-data", + "nonexistent", + "", + ]; + + for key in keys { + assert_eq!(is_internal_key(key), is_internal_key_ref(key), "is_internal_key mismatch for {key:?}"); + assert_eq!( + strip_internal_prefix(key), + strip_internal_prefix_ref(key), + "strip_internal_prefix mismatch for {key:?}" + ); + for suffix in suffixes { + assert_eq!( + has_internal_suffix(key, suffix), + has_internal_suffix_ref(key, suffix), + "has_internal_suffix mismatch for key {key:?} suffix {suffix:?}" + ); + } + } + } + + #[test] + fn test_has_internal_suffix_kelvin_sign_equivalence() { + // U+212A KELVIN SIGN Unicode-lowercases to ASCII 'k'. The optimized ASCII fast path must + // fall back to full Unicode lowercasing for non-ASCII keys so it stays byte-for-byte + // equivalent to the original `key.to_lowercase()` implementation. + let key = "x-rustfs-internal-tier-free-mar\u{212A}er"; + assert!(has_internal_suffix(key, SUFFIX_TIER_FV_MARKER)); + assert_eq!( + has_internal_suffix(key, SUFFIX_TIER_FV_MARKER), + has_internal_suffix_ref(key, SUFFIX_TIER_FV_MARKER) + ); + // A plain ASCII 'k' key still matches, and a non-matching suffix still fails. + assert!(has_internal_suffix("x-rustfs-internal-tier-free-marker", SUFFIX_TIER_FV_MARKER)); + assert!(!has_internal_suffix(key, SUFFIX_COMPRESSION)); + } + + #[test] + fn test_get_str_case_insensitive_fallback() { + // Non-canonical mixed-case key must still be found via the case-insensitive scan. + let metadata = HashMap::from([("X-RustFS-Internal-Compression".to_string(), "s2".to_string())]); + assert_eq!(get_str(&metadata, SUFFIX_COMPRESSION).as_deref(), Some("s2")); + assert!(contains_key_str(&metadata, SUFFIX_COMPRESSION)); + } + + #[test] + fn test_get_bytes_no_case_insensitive_fallback() { + // get_bytes only checks the two canonical keys (matching the original behavior): a + // mixed-case key is not matched. + let meta_sys = HashMap::from([("X-RustFS-Internal-Crc".to_string(), b"z".to_vec())]); + assert_eq!(get_bytes(&meta_sys, SUFFIX_CRC), None); + + let meta_sys = HashMap::from([(internal_key_rustfs(SUFFIX_CRC), b"z".to_vec())]); + assert_eq!(get_bytes(&meta_sys, SUFFIX_CRC), Some(b"z".to_vec())); + } + + #[test] + fn test_long_suffix_falls_back_to_heap_key() { + // A suffix longer than the stack buffer must still round-trip via the heap fallback. + let long_suffix = "x".repeat(INTERNAL_KEY_STACK_CAP); + let mut meta_sys = HashMap::new(); + insert_bytes(&mut meta_sys, &long_suffix, b"payload".to_vec()); + + assert!(contains_key_bytes(&meta_sys, &long_suffix)); + assert_eq!(get_bytes(&meta_sys, &long_suffix), Some(b"payload".to_vec())); + remove_bytes(&mut meta_sys, &long_suffix); + assert!(!contains_key_bytes(&meta_sys, &long_suffix)); + } }