Experiment: avoid locks for empty xsi:type registries

.agents/skills/polyxml-core-engine/SKILL.md

@@ -267,6 +267,13 @@ randomized hasher and an eviction nonce; FIFO would miss on every access for a
 cycle of 17 patterns. Public schema metadata remains mutable before sharing,
 so avoid cached flags that become stale after edits.
 
+The `xsi:type` variant registry shares an atomic populated flag with its locked
+vector across schema clones. Update the flag under the write lock in
+`set_variants`, including when clearing the registry; empty dispatch checks can
+then avoid a read lock. Nonempty vector snapshots and QName lookups remain
+locked. Preserve sequential clone/update tests, concurrent snapshots, and full
+`test_xsi_type` dispatch/error/round-trip coverage when changing the registry.
+
 The serializer writes directly into an append-only byte vector and checks for
 lexical-list obligations before traversing repeated values. Ordinary nested
 records validate their own fields. Lexical lists append tokens to one string;

.agents/skills/polyxml-runtime-investigation/SKILL.md

@@ -23,6 +23,19 @@ separate from the schema-driven `PolyValue` runtime.
   synchronize and rebuild an editable binding. After preparing its environment,
   use `uv run --no-sync` for companion runs. If a build overlaps timing, retain
   and mark that attempt as contaminated, then repeat it.
+  Use worktree-owned workspace build targets for tests, CLI and native bindings.
+  A shared debug target can retain an unchanged test executable linked to an
+  earlier worktree implementation, as well as stale top-level CLI aliases.
+  Even a Cargo compile message does not prove every executable was relinked.
+  Verify discriminating fixtures or symbols when switching revisions; discard
+  mismatched preflights and rerun in an isolated target. Standalone comparison
+  consumers use separate baseline/current targets, explicit dependency paths
+  and Cargo-reported hashed executable artifacts; keep them separate from
+  workspace build caches and retain artifact provenance.
+- Preflight the exact PATH used by the quality gate as well as timing tools.
+  A locally installed TypeScript compiler needs its node_modules/.bin on PATH;
+  having Node available does not make tsc available. Record missing-tool failures
+  and rerun the complete gate with the prepared toolchain environment.
 - Run heavy work serially, with one Cargo worker and the repository memory cap.
   Short comparisons screen hypotheses. Repeat retained improvements with longer
   measurements, alternating revision order and preserving every raw sample.
@@ -35,6 +48,10 @@ separate from the schema-driven `PolyValue` runtime.
 - Preserve validation, errors, split Text/CData/GeneralRef handling, nil reads,
   nesting and metadata mutation semantics. Borrow scalar metadata from the schema
   owned by a frame instead of cloning rich scalar definitions per element.
+  Profile empty `xsi:type` registry checks too: a shared flag maintained by the
+  registration setter can avoid locks without caching mutable field metadata.
+  Compare an empty registry with actual populated dispatch using `variants.rs`;
+  do not assume a plain-record gain also proves polymorphic throughput.
 - Include the 17-pattern cycle when changing eviction: a 16-entry FIFO has
   systematic misses, while salted victim selection preserves reuse at the same
   capacity. Confirm hot cases too; pressure improvements alone do not establish

benchmarks/rust-runtime-investigation/README.md

@@ -113,3 +113,9 @@ For the generated sensor control, run the XML/Serde regression runner and use
 its output directory. This computes each process's median of seven timing
 samples, then the median across processes; it retains the process deltas. The
 statistic differs from the Criterion process means above.
+
+`variants.rs` compares 1,000 ordinary nested items with 1,000 items selected by
+namespaced `xsi:type`. It verifies every scalar payload and the concrete schema,
+then checks the XML round trip before measuring. Use the alternate `--harness`
+argument to check both empty-registry and populated-dispatch paths when changing
+registry synchronization.

benchmarks/rust-runtime-investigation/variants.rs

@@ -0,0 +1,85 @@
+use criterion::{
+    criterion_group, criterion_main, BenchmarkId, Criterion, SamplingMode, Throughput,
+};
+use polyxml::schema::{FieldKind, FieldSchema, ModelSchema, ScalarType, ValueType};
+use polyxml::{deserialize, serialize, PolyValue};
+use std::{hint::black_box, sync::Arc};
+
+fn model(name: &str, namespace: Option<&str>) -> Arc<ModelSchema> {
+    let mut builder = ModelSchema::builder(name).field(FieldSchema::new(
+        "value",
+        b"Value",
+        FieldKind::Element,
+        ValueType::Scalar(ScalarType::Int),
+    ));
+    if let Some(namespace) = namespace {
+        builder = builder.namespace(namespace);
+    }
+    builder.build()
+}
+
+fn fixture(registered: bool, count: usize) -> (Arc<ModelSchema>, Vec<u8>) {
+    let base = model("Base", None);
+    if registered {
+        base.set_variants(vec![model("Derived", Some("urn:derived"))]);
+    }
+    let schema = ModelSchema::builder("Root")
+        .field(FieldSchema::new(
+            "items",
+            b"Item",
+            FieldKind::Element,
+            ValueType::List(Box::new(ValueType::Nested(base))),
+        ))
+        .build();
+    let mut xml = String::from(
+        "<Root xmlns:i='http://www.w3.org/2001/XMLSchema-instance' xmlns:d='urn:derived'>",
+    );
+    for index in 0..count {
+        let selector = if registered {
+            " i:type='d:Derived'"
+        } else {
+            ""
+        };
+        xml.push_str(&format!("<Item{selector}><Value>{index}</Value></Item>"));
+    }
+    xml.push_str("</Root>");
+    let value = deserialize(xml.as_bytes(), Arc::clone(&schema)).unwrap();
+    let items = value.get("items").unwrap().as_list().unwrap();
+    assert_eq!(items.len(), count);
+    for (index, item) in items.iter().enumerate() {
+        assert_eq!(item.get("value"), Some(&PolyValue::Int(index as i64)));
+        let PolyValue::Record { schema, .. } = item else {
+            panic!("expected record")
+        };
+        assert_eq!(schema.name, if registered { "Derived" } else { "Base" });
+    }
+    let output = serialize("Root", &value, &schema, None).unwrap();
+    assert_eq!(value, deserialize(&output, Arc::clone(&schema)).unwrap());
+    (schema, xml.into_bytes())
+}
+
+fn benchmarks(c: &mut Criterion) {
+    for operation in ["variant_read", "variant_write"] {
+        let mut group = c.benchmark_group(operation);
+        group.sampling_mode(SamplingMode::Flat);
+        for (kind, registered) in [("empty_registry", false), ("registered", true)] {
+            let (schema, xml) = fixture(registered, 1000);
+            let value = deserialize(&xml, Arc::clone(&schema)).unwrap();
+            group.throughput(Throughput::Bytes(xml.len() as u64));
+            group.bench_function(BenchmarkId::new(kind, 1000), |b| {
+                if operation == "variant_read" {
+                    b.iter(|| {
+                        black_box(deserialize(black_box(&xml), Arc::clone(&schema)).unwrap())
+                    });
+                } else {
+                    b.iter(|| {
+                        black_box(serialize("Root", black_box(&value), &schema, None).unwrap())
+                    });
+                }
+            });
+        }
+        group.finish();
+    }
+}
+criterion_group!(benches, benchmarks);
+criterion_main!(benches);

crates/polyxml-core/src/schema.rs

@@ -1,4 +1,5 @@
 use std::collections::HashMap;
+use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{Arc, RwLock};
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -106,6 +107,12 @@ impl FieldSchema {
     }
 }
 
+#[derive(Debug, Default)]
+struct VariantRegistry {
+    schemas: RwLock<Vec<Arc<ModelSchema>>>,
+    populated: AtomicBool,
+}
+
 #[derive(Debug, Clone)]
 pub struct ModelSchema {
     pub name: String,
@@ -125,7 +132,7 @@ pub struct ModelSchema {
     pub content_pattern: Option<regex::Regex>,
     /// Concrete derivations eligible for `xsi:type` dispatch. Python may
     /// refresh this registry when subclasses are defined after first use.
-    variants: Arc<RwLock<Vec<Arc<ModelSchema>>>>,
+    variants: Arc<VariantRegistry>,
 }
 
 impl ModelSchema {
@@ -136,29 +143,44 @@ impl ModelSchema {
     /// Register the concrete derivations eligible for `xsi:type` dispatch.
     /// Replaces the registry when subclasses are discovered after first use.
     pub fn set_variants(&self, variants: Vec<Arc<ModelSchema>>) {
-        *self.variants.write().unwrap_or_else(|p| p.into_inner()) = variants;
+        let mut registered = self
+            .variants
+            .schemas
+            .write()
+            .unwrap_or_else(|p| p.into_inner());
+        *registered = variants;
+        // Publish emptiness while holding the registry lock. Cloned schemas
+        // share this flag, so later registration/clearing cannot leave a stale
+        // per-schema dispatch plan. Nonempty lookups still use the lock.
+        self.variants
+            .populated
+            .store(!registered.is_empty(), Ordering::Release);
     }
 
     /// Registered derivations for `xsi:type` dispatch (empty when none).
     pub fn variants(&self) -> Vec<Arc<ModelSchema>> {
+        if !self.has_variants() {
+            return Vec::new();
+        }
         self.variants
+            .schemas
             .read()
             .unwrap_or_else(|p| p.into_inner())
             .clone()
     }
 
     /// Whether any `xsi:type` derivations are registered for this type.
     pub fn has_variants(&self) -> bool {
-        !self
-            .variants
-            .read()
-            .unwrap_or_else(|p| p.into_inner())
-            .is_empty()
+        self.variants.populated.load(Ordering::Acquire)
     }
 
     /// Look up a derivation by the QName local part of an `xsi:type` value.
     pub fn find_variant(&self, local: &[u8]) -> Option<Arc<ModelSchema>> {
+        if !self.has_variants() {
+            return None;
+        }
         self.variants
+            .schemas
             .read()
             .unwrap_or_else(|p| p.into_inner())
             .iter()
@@ -168,7 +190,11 @@ impl ModelSchema {
 
     /// Look up a derivation by its complete QName.
     pub fn find_variant_qname(&self, namespace: &str, local: &[u8]) -> Option<Arc<ModelSchema>> {
+        if !self.has_variants() {
+            return None;
+        }
         self.variants
+            .schemas
             .read()
             .unwrap_or_else(|p| p.into_inner())
             .iter()
@@ -180,7 +206,11 @@ impl ModelSchema {
 
     /// Whether `candidate` is a registered derivation of `self`.
     pub fn matches_variant(&self, candidate: &ModelSchema) -> bool {
+        if !self.has_variants() {
+            return false;
+        }
         self.variants
+            .schemas
             .read()
             .unwrap_or_else(|p| p.into_inner())
             .iter()
@@ -655,7 +685,7 @@ impl ModelSchemaBuilder {
             is_abstract: self.is_abstract,
             strict_root: self.strict_root,
             content_pattern: self.content_pattern,
-            variants: Arc::new(RwLock::new(Vec::new())),
+            variants: Arc::new(VariantRegistry::default()),
         })
     }
 }

crates/polyxml-core/tests/test_variant_registry.rs

@@ -0,0 +1,75 @@
+use std::sync::Arc;
+
+use polyxml::schema::ModelSchema;
+
+#[test]
+fn registration_and_clearing_are_visible_to_schema_clones() {
+    let base = ModelSchema::builder("Base").build();
+    let clone = (*base).clone();
+    let derived = ModelSchema::builder("Derived")
+        .namespace("urn:derived")
+        .build();
+    assert!(!clone.has_variants());
+    assert!(clone.variants().is_empty());
+    base.set_variants(vec![Arc::clone(&derived)]);
+    assert!(clone.has_variants());
+    assert!(Arc::ptr_eq(&clone.variants()[0], &derived));
+    assert!(Arc::ptr_eq(
+        &clone.find_variant(b"Derived").unwrap(),
+        &derived
+    ));
+    assert!(clone.find_variant_qname("urn:other", b"Derived").is_none());
+    assert!(Arc::ptr_eq(
+        &clone.find_variant_qname("urn:derived", b"Derived").unwrap(),
+        &derived
+    ));
+    assert!(clone.matches_variant(&derived));
+    clone.set_variants(Vec::new());
+    assert!(!base.has_variants());
+    assert!(base.variants().is_empty());
+    assert!(base.find_variant(b"Derived").is_none());
+    assert!(!base.matches_variant(&derived));
+    clone.set_variants(vec![Arc::clone(&derived)]);
+    assert!(base.has_variants());
+    assert!(base.matches_variant(&derived));
+}
+
+#[test]
+fn concurrent_registry_updates_keep_complete_variant_snapshots() {
+    let base = ModelSchema::builder("Base").build();
+    let derived = ModelSchema::builder("Derived").build();
+    std::thread::scope(|scope| {
+        scope.spawn(|| {
+            for index in 0..4096 {
+                base.set_variants(if index % 2 == 0 {
+                    Vec::new()
+                } else {
+                    vec![Arc::clone(&derived)]
+                });
+            }
+            base.set_variants(vec![Arc::clone(&derived)]);
+        });
+        for _ in 0..4 {
+            scope.spawn(|| {
+                for _ in 0..4096 {
+                    // These calls can observe different registry revisions.
+                    // Each populated snapshot must contain the original Arc.
+                    let _ = base.has_variants();
+                    let variants = base.variants();
+                    assert!(variants.len() <= 1);
+                    for variant in variants {
+                        assert!(Arc::ptr_eq(&variant, &derived));
+                    }
+                    if let Some(variant) = base.find_variant(b"Derived") {
+                        assert!(Arc::ptr_eq(&variant, &derived));
+                    }
+                }
+            });
+        }
+    });
+    assert!(base.has_variants());
+    assert!(Arc::ptr_eq(&base.variants()[0], &derived));
+    base.set_variants(Vec::new());
+    assert!(!base.has_variants());
+    assert!(base.variants().is_empty());
+}