perf(themes): PR #120 round 5 -- O(1) frozen counts via store totals
The round-4 freeze read counted the retained for_uri vector inside status_facts_msg -- correct, but StatusFacts runs at frame cadence for every semantic session under the shared store mutex, so a long stale interval cost O(frames x diagnostics x sessions). DiagnosticStore now maintains per-URI severity totals alongside by_uri: set replaces them (an empty publication removes them with the vector), clear removes them, and mark_stale deliberately preserves both -- entries exist exactly when by_uri entries do, and set/clear are the store's only by_uri mutators. status_facts_msg reads the tuple in O(1), and the all-URI severity_totals sum reuses the cached tuples. Store unit (acceptance item 34) pins the invariant: all four totals replace correctly, survive staleness, and clear with the diagnostic vector; empty_set_clears_uri asserts the totals drop too. The rounds 3-4 freeze acceptance passes unchanged -- behavior parity, so the unit pin is the evidence (no runtime bite exists for a behavior-preserving refactor). Round-5 implementation user-authored; this commit folds it with framing revision 9, the protocol doc's O(1) note, and the acceptance manifest pointer to the item-34 unit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VoiEyuPjoBhvwACf8HAnLB
This commit is contained in:
parent
3d6336a912
commit
b75d45b1d4
|
|
@ -132,7 +132,9 @@ instance's stale-store diagnostic-count freeze is store knowledge,
|
||||||
not session state — the re-sent `StatusFacts` after a snapshot
|
not session state — the re-sent `StatusFacts` after a snapshot
|
||||||
carries the frozen counts, never zeros, including for a session
|
carries the frozen counts, never zeros, including for a session
|
||||||
whose first frame lands during staleness (a late joiner attaching
|
whose first frame lands during staleness (a late joiner attaching
|
||||||
mid-edit).
|
mid-edit). The store maintains those per-URI severity totals with
|
||||||
|
the retained vector, so frame-time producers read them in O(1)
|
||||||
|
rather than rescanning diagnostics for every attached session.
|
||||||
|
|
||||||
## Capability and version mechanics
|
## Capability and version mechanics
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,18 @@
|
||||||
# Theme faces — framing (Arc 4 stage 1, themes)
|
# Theme faces — framing (Arc 4 stage 1, themes)
|
||||||
|
|
||||||
**Revision 8 — 2026-07-15. Status: implemented on branch
|
**Revision 9 — 2026-07-15. Status: implemented on branch
|
||||||
`theme-faces` (PR #120); revision 8 folds PR round 4.**
|
`theme-faces` (PR #120); revision 9 folds PR round 5.**
|
||||||
|
|
||||||
|
Revision 9 (PR #120 round 5, one finding): the store-sourced freeze
|
||||||
|
no longer rescans the retained diagnostic vector at frame cadence.
|
||||||
|
`DiagnosticStore` maintains per-URI severity totals alongside
|
||||||
|
`by_uri`, replacing or removing both in `set` / `clear` while
|
||||||
|
`mark_stale` deliberately preserves both. `status_facts_msg` reads
|
||||||
|
that tuple in O(1), so the round-4 late-join correctness does not
|
||||||
|
turn a long stale interval into O(frames x diagnostics x semantic
|
||||||
|
sessions) work under the shared store mutex. Acceptance item 34 is
|
||||||
|
the store unit pin: all four totals replace correctly, survive
|
||||||
|
staleness, and clear with the diagnostic vector.
|
||||||
|
|
||||||
Revision 8 (PR #120 round 4, one finding): the diagnostic-count
|
Revision 8 (PR #120 round 4, one finding): the diagnostic-count
|
||||||
freeze is sourced from the diag store itself, superseding round 3's
|
freeze is sourced from the diag store itself, superseding round 3's
|
||||||
|
|
@ -671,10 +682,13 @@ pub struct ThemeFace {
|
||||||
other buffers' baselines (each buffer's own snapshot precedes its
|
other buffers' baselines (each buffer's own snapshot precedes its
|
||||||
revisit). The diagnostic-count freeze needs no protection here
|
revisit). The diagnostic-count freeze needs no protection here
|
||||||
(rounds 3–4): it is sourced from the diag store's retained vector
|
(rounds 3–4): it is sourced from the diag store's retained vector
|
||||||
— `mark_stale` keeps the last published diagnostics, whose counts
|
and its cached per-URI severity tuple — `mark_stale` keeps the last
|
||||||
merely lag — never from session state, so the reset cannot zero
|
published diagnostics and counts, which merely lag — never from
|
||||||
mid-edit counts and the freeze holds even for a session attaching
|
session state, so the reset cannot zero mid-edit counts and the
|
||||||
during staleness. Resetting on a failed write is harmless — the
|
freeze holds even for a session attaching during staleness. The
|
||||||
|
producer reads the tuple in O(1), rather than scanning the retained
|
||||||
|
vector for every semantic session at frame cadence (round 5).
|
||||||
|
Resetting on a failed write is harmless — the
|
||||||
failure mode is one redundant re-send, never staleness. The GPU's `BufferSnapshot` arm mirrors the contract
|
failure mode is one redundant re-send, never staleness. The GPU's `BufferSnapshot` arm mirrors the contract
|
||||||
(round 3 finding 1): it clears its buffer-scoped facts —
|
(round 3 finding 1): it clears its buffer-scoped facts —
|
||||||
search/menu popups (which gate key and pointer interception) and
|
search/menu popups (which gate key and pointer interception) and
|
||||||
|
|
@ -1016,3 +1030,9 @@ Keybinding-driven tests dispatch keys, never `pmacs.command.invoke`.
|
||||||
`SemanticRenderState` — the session's first frame reports the
|
`SemanticRenderState` — the session's first frame reports the
|
||||||
store's preserved counts, because the freeze is the retained
|
store's preserved counts, because the freeze is the retained
|
||||||
diagnostic vector itself, not a per-session cache.
|
diagnostic vector itself, not a per-session cache.
|
||||||
|
34. **Frozen counts stay O(1) at frame cadence** (PR round 5, Rust
|
||||||
|
unit): `DiagnosticStore` caches all four per-URI severity totals;
|
||||||
|
replacement updates them, `mark_stale` preserves them, and
|
||||||
|
`clear` / an empty replacement remove them with the diagnostic
|
||||||
|
vector. `StatusFacts` reads this tuple rather than rescanning
|
||||||
|
`for_uri` for every semantic session on every frame.
|
||||||
|
|
|
||||||
82
src/diag.rs
82
src/diag.rs
|
|
@ -196,6 +196,10 @@ impl Diagnostic {
|
||||||
#[derive(Default)]
|
#[derive(Default)]
|
||||||
pub struct DiagnosticStore {
|
pub struct DiagnosticStore {
|
||||||
by_uri: HashMap<String, Vec<Diagnostic>>,
|
by_uri: HashMap<String, Vec<Diagnostic>>,
|
||||||
|
/// Per-URI severity totals, maintained alongside `by_uri` so
|
||||||
|
/// frame-time consumers do not rescan every diagnostic at render
|
||||||
|
/// cadence. Entries exist exactly when `by_uri` entries do.
|
||||||
|
severity_counts: HashMap<String, (u32, u32, u32, u32)>,
|
||||||
/// URIs whose stored diagnostics are known to be out of date
|
/// URIs whose stored diagnostics are known to be out of date
|
||||||
/// because a `textDocument/didChange` was issued after the last
|
/// because a `textDocument/didChange` was issued after the last
|
||||||
/// `publishDiagnostics` was absorbed. `Self::set` clears entries
|
/// `publishDiagnostics` was absorbed. `Self::set` clears entries
|
||||||
|
|
@ -210,6 +214,20 @@ pub struct DiagnosticStore {
|
||||||
epochs: HashMap<String, u64>,
|
epochs: HashMap<String, u64>,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn count_severities(diags: &[Diagnostic]) -> (u32, u32, u32, u32) {
|
||||||
|
let mut counts = (0u32, 0u32, 0u32, 0u32);
|
||||||
|
for diagnostic in diags {
|
||||||
|
let slot = match diagnostic.severity {
|
||||||
|
DiagnosticSeverity::Error => &mut counts.0,
|
||||||
|
DiagnosticSeverity::Warning => &mut counts.1,
|
||||||
|
DiagnosticSeverity::Information => &mut counts.2,
|
||||||
|
DiagnosticSeverity::Hint => &mut counts.3,
|
||||||
|
};
|
||||||
|
*slot = slot.saturating_add(1);
|
||||||
|
}
|
||||||
|
counts
|
||||||
|
}
|
||||||
|
|
||||||
impl DiagnosticStore {
|
impl DiagnosticStore {
|
||||||
/// Empty store.
|
/// Empty store.
|
||||||
#[must_use]
|
#[must_use]
|
||||||
|
|
@ -227,11 +245,14 @@ impl DiagnosticStore {
|
||||||
pub fn set(&mut self, uri: impl Into<String>, mut diags: Vec<Diagnostic>) {
|
pub fn set(&mut self, uri: impl Into<String>, mut diags: Vec<Diagnostic>) {
|
||||||
diags.sort_by(Diagnostic::compare_by_position);
|
diags.sort_by(Diagnostic::compare_by_position);
|
||||||
let uri = uri.into();
|
let uri = uri.into();
|
||||||
|
let counts = count_severities(&diags);
|
||||||
self.stale_uris.remove(&uri);
|
self.stale_uris.remove(&uri);
|
||||||
*self.epochs.entry(uri.clone()).or_insert(0) += 1;
|
*self.epochs.entry(uri.clone()).or_insert(0) += 1;
|
||||||
if diags.is_empty() {
|
if diags.is_empty() {
|
||||||
self.by_uri.remove(&uri);
|
self.by_uri.remove(&uri);
|
||||||
|
self.severity_counts.remove(&uri);
|
||||||
} else {
|
} else {
|
||||||
|
self.severity_counts.insert(uri.clone(), counts);
|
||||||
self.by_uri.insert(uri, diags);
|
self.by_uri.insert(uri, diags);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -240,6 +261,7 @@ impl DiagnosticStore {
|
||||||
/// no entry to be stale about.
|
/// no entry to be stale about.
|
||||||
pub fn clear(&mut self, uri: &str) {
|
pub fn clear(&mut self, uri: &str) {
|
||||||
self.by_uri.remove(uri);
|
self.by_uri.remove(uri);
|
||||||
|
self.severity_counts.remove(uri);
|
||||||
self.stale_uris.remove(uri);
|
self.stale_uris.remove(uri);
|
||||||
*self.epochs.entry(uri.to_owned()).or_insert(0) += 1;
|
*self.epochs.entry(uri.to_owned()).or_insert(0) += 1;
|
||||||
}
|
}
|
||||||
|
|
@ -285,19 +307,26 @@ impl DiagnosticStore {
|
||||||
let mut w = 0;
|
let mut w = 0;
|
||||||
let mut i = 0;
|
let mut i = 0;
|
||||||
let mut h = 0;
|
let mut h = 0;
|
||||||
for diags in self.by_uri.values() {
|
for &(errors, warnings, information, hints) in self.severity_counts.values() {
|
||||||
for d in diags {
|
e += errors as usize;
|
||||||
match d.severity {
|
w += warnings as usize;
|
||||||
DiagnosticSeverity::Error => e += 1,
|
i += information as usize;
|
||||||
DiagnosticSeverity::Warning => w += 1,
|
h += hints as usize;
|
||||||
DiagnosticSeverity::Information => i += 1,
|
|
||||||
DiagnosticSeverity::Hint => h += 1,
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
(e, w, i, h)
|
(e, w, i, h)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Per-URI totals `(error, warning, information, hint)`.
|
||||||
|
///
|
||||||
|
/// The tuple is computed once by [`Self::set`] and deliberately
|
||||||
|
/// survives [`Self::mark_stale`]: staleness invalidates byte
|
||||||
|
/// positions, while the last published counts remain valid as a
|
||||||
|
/// frozen status summary until the next publication.
|
||||||
|
#[must_use]
|
||||||
|
pub fn severity_counts_for(&self, uri: &str) -> (u32, u32, u32, u32) {
|
||||||
|
self.severity_counts.get(uri).copied().unwrap_or_default()
|
||||||
|
}
|
||||||
|
|
||||||
/// Per-URI count.
|
/// Per-URI count.
|
||||||
#[must_use]
|
#[must_use]
|
||||||
pub fn count_for(&self, uri: &str) -> usize {
|
pub fn count_for(&self, uri: &str) -> usize {
|
||||||
|
|
@ -860,12 +889,47 @@ mod tests {
|
||||||
assert_eq!(totals, (2, 1, 1, 1));
|
assert_eq!(totals, (2, 1, 1, 1));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn cached_severity_counts_replace_clear_and_survive_staleness() {
|
||||||
|
let mut store = DiagnosticStore::new();
|
||||||
|
assert_eq!(store.severity_counts_for("file:///a"), (0, 0, 0, 0));
|
||||||
|
|
||||||
|
store.set(
|
||||||
|
"file:///a",
|
||||||
|
vec![
|
||||||
|
diag(0, DiagnosticSeverity::Error, "1"),
|
||||||
|
diag(1, DiagnosticSeverity::Error, "2"),
|
||||||
|
diag(2, DiagnosticSeverity::Warning, "3"),
|
||||||
|
diag(3, DiagnosticSeverity::Information, "4"),
|
||||||
|
diag(4, DiagnosticSeverity::Hint, "5"),
|
||||||
|
],
|
||||||
|
);
|
||||||
|
assert_eq!(store.severity_counts_for("file:///a"), (2, 1, 1, 1));
|
||||||
|
|
||||||
|
store.mark_stale("file:///a");
|
||||||
|
assert_eq!(
|
||||||
|
store.severity_counts_for("file:///a"),
|
||||||
|
(2, 1, 1, 1),
|
||||||
|
"staleness freezes the last published counts"
|
||||||
|
);
|
||||||
|
|
||||||
|
store.set(
|
||||||
|
"file:///a",
|
||||||
|
vec![diag(0, DiagnosticSeverity::Warning, "replacement")],
|
||||||
|
);
|
||||||
|
assert_eq!(store.severity_counts_for("file:///a"), (0, 1, 0, 0));
|
||||||
|
|
||||||
|
store.clear("file:///a");
|
||||||
|
assert_eq!(store.severity_counts_for("file:///a"), (0, 0, 0, 0));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn empty_set_clears_uri() {
|
fn empty_set_clears_uri() {
|
||||||
let mut s = DiagnosticStore::new();
|
let mut s = DiagnosticStore::new();
|
||||||
s.set("a", vec![diag(0, DiagnosticSeverity::Error, "x")]);
|
s.set("a", vec![diag(0, DiagnosticSeverity::Error, "x")]);
|
||||||
s.set("a", Vec::new());
|
s.set("a", Vec::new());
|
||||||
assert!(s.for_uri("a").is_empty());
|
assert!(s.for_uri("a").is_empty());
|
||||||
|
assert_eq!(s.severity_counts_for("a"), (0, 0, 0, 0));
|
||||||
assert_eq!(s.uris().count(), 0);
|
assert_eq!(s.uris().count(), 0);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -889,12 +889,12 @@ impl SemanticRenderState {
|
||||||
/// last published value while the diag store is stale — mid-edit
|
/// last published value while the diag store is stale — mid-edit
|
||||||
/// positions are wrong but *counts* merely lag, and flickering
|
/// positions are wrong but *counts* merely lag, and flickering
|
||||||
/// to zero on every keystroke would be worse. The freeze IS the
|
/// to zero on every keystroke would be worse. The freeze IS the
|
||||||
/// store's retained vector (rounds 3–4): `mark_stale` keeps the
|
/// store's retained state (rounds 3–5): `mark_stale` keeps the
|
||||||
/// last published diagnostics, so counting them while stale
|
/// last published diagnostics and their cached severity totals,
|
||||||
/// yields the frozen value with no session state to lose — not
|
/// so reading the totals while stale yields the frozen value in
|
||||||
/// to a snapshot reset, and not by attaching mid-edit. The
|
/// O(1) with no session state to lose — not to a snapshot reset,
|
||||||
/// daemon's write loop keeps the variant off wires negotiated
|
/// and not by attaching mid-edit. The daemon's write loop keeps
|
||||||
/// `< 8`.
|
/// the variant off wires negotiated `< 8`.
|
||||||
fn status_facts_msg(
|
fn status_facts_msg(
|
||||||
&mut self,
|
&mut self,
|
||||||
state: &EditorState,
|
state: &EditorState,
|
||||||
|
|
@ -917,25 +917,22 @@ impl SemanticRenderState {
|
||||||
buffer_file_uri(&core, buffer_id).map_or((0, 0), |uri| {
|
buffer_file_uri(&core, buffer_id).map_or((0, 0), |uri| {
|
||||||
let store = state.lsp_manager.borrow().diag_store();
|
let store = state.lsp_manager.borrow().diag_store();
|
||||||
let guard = store.lock().expect("diag store mutex poisoned");
|
let guard = store.lock().expect("diag store mutex poisoned");
|
||||||
// Counted even while the store is STALE (round 4):
|
// Read even while the store is STALE (round 4):
|
||||||
// `mark_stale` keeps the last published vector (T
|
// `mark_stale` keeps the last published diagnostics
|
||||||
// M11.8) — positions are invalid mid-edit, but counts
|
// (T M11.8) — positions are invalid mid-edit, but
|
||||||
// merely lag, so the retained entries ARE the frozen
|
// counts merely lag, so the retained totals ARE the
|
||||||
// value. Sourcing the freeze from the store rather
|
// frozen value. Sourcing the freeze from the store
|
||||||
// than any per-session cache means a session first
|
// rather than any per-session cache means a session
|
||||||
// rendering during staleness — a late joiner, or a
|
// first rendering during staleness — a late joiner,
|
||||||
// buffer first visited mid-edit — reports the
|
// or a buffer first visited mid-edit — reports the
|
||||||
// preserved counts instead of zeros, and the snapshot
|
// preserved counts instead of zeros, and the snapshot
|
||||||
// reset has nothing count-related to preserve.
|
// reset has nothing count-related to preserve.
|
||||||
let mut errors = 0u32;
|
// `DiagnosticStore::set` computes this tuple once
|
||||||
let mut warnings = 0u32;
|
// (round 5). StatusFacts runs at frame cadence for
|
||||||
for d in guard.for_uri(&uri) {
|
// every semantic session, so rescanning the retained
|
||||||
match d.severity {
|
// vector here would make stale intervals
|
||||||
crate::diag::DiagnosticSeverity::Error => errors += 1,
|
// O(frames * diagnostics).
|
||||||
crate::diag::DiagnosticSeverity::Warning => warnings += 1,
|
let (errors, warnings, _, _) = guard.severity_counts_for(&uri);
|
||||||
_ => {}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
(errors, warnings)
|
(errors, warnings)
|
||||||
})
|
})
|
||||||
};
|
};
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,8 @@
|
||||||
// theme_faces_acceptance.rs --- Themes Arc 4 stage 1 acceptance
|
// theme_faces_acceptance.rs --- Themes Arc 4 stage 1 acceptance
|
||||||
// (docs/theme-faces-framing.md, acceptance items 1–19, 24–26, 28–29,
|
// (docs/theme-faces-framing.md, acceptance items 1–19, 24–26, 28–29,
|
||||||
// and 32–33; the GPU routes — 20–23, 27, and 30–31 — live in
|
// and 32–33; the GPU routes — 20–23, 27, and 30–31 — live in
|
||||||
// pmacs-gpu's headless suite).
|
// pmacs-gpu's headless suite; item 34 is a `DiagnosticStore` unit in
|
||||||
|
// src/diag.rs).
|
||||||
|
|
||||||
//! Named UI faces (`ui` / `ui.*` theme entries) + the `ThemeFacts`
|
//! Named UI faces (`ui` / `ui.*` theme entries) + the `ThemeFacts`
|
||||||
//! wire channel (protocol v16).
|
//! wire channel (protocol v16).
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue