Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions .github/workflows/avatar-restore.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,16 @@ jobs:
text = mod.read_text()
marker = 'mod native;'
assert marker in text
mod.write_text(text.replace(marker, marker + '\n#[cfg(all(test, not(target_family = "wasm")))]\nmod probe;', 1))
if 'mod probe;' not in text:
mod.write_text(text.replace(marker, marker + '\n#[cfg(all(test, not(target_family = "wasm")))]\nmod probe;', 1))
native = root / 'native.rs'
text = native.read_text()
marker = 'pub async fn read_persistent(key: &str) -> Option<Vec<u8>> {'
assert marker in text
text = text.replace(marker, marker + '\n #[cfg(test)]\n PERSISTENT_READS.fetch_add(1, std::sync::atomic::Ordering::Relaxed);', 1)
text += '\n#[cfg(test)]\npub(super) static PERSISTENT_READS: portable_atomic::AtomicU64 = portable_atomic::AtomicU64::new(0);\n'
if 'PERSISTENT_READS.fetch_add' not in text:
text = text.replace(marker, marker + '\n #[cfg(test)]\n PERSISTENT_READS.fetch_add(1, std::sync::atomic::Ordering::Relaxed);', 1)
if 'static PERSISTENT_READS:' not in text:
text += '\n#[cfg(test)]\npub(super) static PERSISTENT_READS: portable_atomic::AtomicU64 = portable_atomic::AtomicU64::new(0);\n'
native.write_text(text)
PY
cargo test -p oxidezap-gui --bin oxidezap --locked native_restore_probe -- --ignored --nocapture
Expand Down
4 changes: 3 additions & 1 deletion crates/gui/src/session/avatar/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,16 @@ mod native;
mod web;
#[cfg(target_family = "wasm")]
mod web_lock;
#[cfg(target_family = "wasm")]
mod web_platform;

#[cfg(all(test, not(target_family = "wasm")))]
mod probe;

#[cfg(not(target_family = "wasm"))]
use native as imp;
#[cfg(target_family = "wasm")]
use web as imp;
use web_platform as imp;

use portable_atomic::{AtomicU64, Ordering};
use std::collections::{HashMap, HashSet};
Expand Down
10 changes: 0 additions & 10 deletions crates/gui/src/session/avatar/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,6 @@ static STORAGE_GENERATION: portable_atomic::AtomicU64 = portable_atomic::AtomicU

static ACTIVE_ACCOUNT: RwLock<Option<oxidezap_core::AccountId>> = RwLock::new(None);

pub(super) async fn decode_avatar(
bytes: Vec<u8>,
_priority: super::DecodePriority,
current: impl FnOnce() -> bool + Send + 'static,
) -> Option<(std::sync::Arc<gpui::RenderImage>, u64)> {
current()
.then(|| crate::session::media::decode_avatar(&bytes))
.flatten()
}

pub fn set_active_account(account: Option<oxidezap_core::AccountId>) {
if let Ok(mut lock) = ACTIVE_ACCOUNT.write() {
if *lock != account {
Expand Down
13 changes: 13 additions & 0 deletions crates/gui/src/session/avatar/web_platform.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//! Web avatar decoding and the persistent storage implementation.

pub use super::web::*;

pub(super) async fn decode_avatar(
bytes: Vec<u8>,
_priority: super::DecodePriority,
current: impl FnOnce() -> bool + Send + 'static,
) -> Option<(std::sync::Arc<gpui::RenderImage>, u64)> {
current()
.then(|| crate::session::media::decode_avatar(&bytes))
.flatten()
}
40 changes: 38 additions & 2 deletions crates/plugin-host/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,15 +285,20 @@ const PONG_WITHOUT_PERMISSION: &str = r#"(module
)"#;

fn spins() -> String {
versioned(SPINS)
// Wasmi charges const operators before folding constant/drop pairs away.
// Burn the real per-call fuel budget with fewer interpreter dispatches:
// a bare branch needs 50 million debug dispatches and races the 10 s
// publication deadline on a busy CI runner. The loop, fuel budget,
// deadline and fuel-trap assertion remain unchanged.
versioned(&SPINS.replace("$FUEL_WORK", &"i32.const 0 drop\n".repeat(64)))
}

const SPINS: &str = r#"(module
(import "oxidezap" "oxi_subscribe" (func $subscribe (param i64)))
(memory (export "memory") 1)
(func (export "oxi_abi_version") (result i32) (i32.const $ABI_VERSION))
(func (export "oxi_init") (result i32) (call $subscribe (i64.const 2)) (i32.const 0))
(func (export "oxi_on_event") (param i32 i32) (result i32) (loop (br 0)) (i32.const 0))
(func (export "oxi_on_event") (param i32 i32) (result i32) (loop $FUEL_WORK (br 0)) (i32.const 0))
)"#;

/// Asks for one page and then for far more than the limit allows.
Expand Down Expand Up @@ -608,6 +613,37 @@ fn a_command_outside_the_declared_capabilities_never_reaches_the_daemon() {
assert!(surface.is_running(), "but it is not stopped for trying");
}

/// Keep the original empty-loop case covered independently of the cheaper
/// host fixture: adding billed work must not hide a free-branch regression.
#[test]
fn a_bare_branch_loop_consumes_fuel() {
let (tx, rx) = std::sync::mpsc::sync_channel(1);
let worker = std::thread::spawn(move || {
let mut config = wasmi::Config::default();
config.consume_fuel(true);
let engine = wasmi::Engine::new(&config);
let bytes = wat::parse_str("(module (func (export \"spin\") (loop (br 0))))")
.expect("valid bare-loop fixture");
let module = wasmi::Module::new(&engine, &bytes[..]).expect("the module validates");
let mut store = wasmi::Store::new(&engine, ());
store.set_fuel(32).expect("fuel metering is enabled");
let instance = wasmi::Linker::new(&engine)
.instantiate_and_start(&mut store, &module)
.expect("the module has no start function");
let spin = instance.get_typed_func::<(), ()>(&store, "spin").unwrap();
let error = spin
.call(&mut store, ())
.expect_err("the loop exhausts fuel");
tx.send(error.as_trap_code()).unwrap();
});
assert_eq!(
rx.recv_timeout(Duration::from_secs(10))
.expect("bare branches must consume fuel"),
Some(wasmi::TrapCode::OutOfFuel)
);
worker.join().expect("the fuel probe completed");
}

/// The number that makes running a stranger's code in this process
/// defensible.
#[test]
Expand Down
125 changes: 125 additions & 0 deletions docs/measurements/ci-288-289-triage.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
# CI failures shared by the profiling PRs

Checkpoint for #288 and #289, against main `33108ce`, core `af2e018c`.
These are baseline issues, independent of the SQL and phone-metadata patches.
No production dependencies, fuel limits, test deadlines or assertions change.

## Web compilation failure

The [#288 web job](https://github.com/oxidezap/client/actions/runs/40998832174/job/123878204949)
and [#289 web job](https://github.com/oxidezap/client/actions/runs/41000719222/job/123884222100)
fail before launching a browser. Both report the same errors:

```text
error[E0433]: cannot find `session` in the crate root
avatar/web.rs:36: crate::session::media::decode_avatar(&bytes)
error[E0425]: cannot find type `DecodePriority` in module `super`
avatar/web.rs:32: _priority: super::DecodePriority
error[E0433]: cannot find module or crate `gpui` in this scope
avatar/web.rs:34: Arc<gpui::RenderImage>
```

`daemon::media::avatar_web_tests` imports the actual GUI storage source by
path so it can exercise Cache Storage and Web Locks without linking GPUI.
#282 added a GUI decode function to that file. The daemon has neither those
GUI modules/types nor a GPUI dependency. Main already has both the import
and the GUI function; neither profiling PR changes them. A clean baseline
compilation reproduced all three errors with the same library-test target:

```sh
RUSTFLAGS='--cfg web_sys_unstable_apis' \
cargo test -p oxidezap-daemon --lib --target wasm32-unknown-unknown --no-run
```

The fix moves only the GUI decode function into `avatar/web_platform.rs`.
The GUI selects that adapter as `imp`; it reexports the existing `web`
storage implementation. The daemon still imports `web.rs`, unchanged at its
call site. Storage implementation bytes are identical after removing the
GUI function. No browser tests, data-isolation checks or storage logic are
removed or stubbed, and the daemon gains no GUI dependency.

## Native fuel test flake

The [#288 Check job](https://github.com/oxidezap/client/actions/runs/40998832174/job/123878205132)
passed workspace Clippy and then failed at:

```text
tests::a_plugin_that_loops_forever_runs_out_of_fuel_and_is_stopped
crates/plugin-host/src/tests.rs:110:13:
timed out waiting for the plugin to be stopped
```

Nextest reported failure after 11.004 seconds. The adjacent self-waking
plugin test took 21.454 seconds. The failed fixture is a bare infinite
branch, draining the real 50,000,000-unit per-call fuel budget one branch
at a time in the debug interpreter. Its publication poll has a ten-second
wall-clock deadline. Both plugin-host sources and its dependency graph are
unchanged by #288; chat-store is not a plugin-host dependency.

The unchanged fixture passed isolated on #288 in 8.44 seconds and on main
in 8.47 seconds. Pinning the main test and one synthetic busy Python process
to the same CPU reproduced the exact timeout; the process finished after
16.75 seconds. The competing process was terminated in a `finally` block.
This establishes a load-sensitive baseline flake, not a SQL regression.

Reproduction uses the test executable reported by Cargo:

```sh
cargo test -p oxidezap-plugin-host --all-features --lib \
tests::a_plugin_that_loops_forever_runs_out_of_fuel_and_is_stopped -- --exact
# To reproduce contention, pin this executable and one CPU-busy process
# to the same allowed CPU; always terminate that process after the test.
```

The fixture now inserts 64 constant/drop pairs inside the same infinite
loop. Wasmi 2.0 meters each constant operator before folding the pair away;
`engine/translator/func/fuel.rs` and `engine/costs.rs` in the locked wasmi
source define this behavior. The loop consumes the same production fuel
budget with fewer interpreter dispatches. Its unconditional branch,
ten-second deadline, stopped-state assertion and fuel-error assertion all
remain. A separate bounded 32-unit probe retains the original bare-branch
case and asserts the precise OutOfFuel trap, so adding other billed work
cannot hide a free-branch regression. No runtime sandbox setting or budget changes.

With the same CPU contention, the adjusted fixture passed five consecutive
runs in 0.60–0.68 seconds. Isolated, it passed in 0.25 seconds. The full
plugin-host suite passed, 136 tests and one existing ignored test. Clippy
for all plugin-host targets/features passed with warnings denied.

## Browser validation

The existing daemon and session library tests were built and run with the
CI flags, stable Rust 1.99.1 and wasm-bindgen-test-runner 0.2.129. A matching
Chromium/ChromeDriver 156.0.8078.73 pair was unpacked into `/tmp` for the
synthetic browser runs; no installed browser or user profile was modified.

```sh
RUSTFLAGS='--cfg web_sys_unstable_apis' \
CARGO_TARGET_WASM32_UNKNOWN_UNKNOWN_RUNNER=wasm-bindgen-test-runner \
cargo test -p oxidezap-daemon -p oxidezap-session --lib \
--target wasm32-unknown-unknown
```

All 16 daemon tests and 32 session tests passed, including the real avatar
Cache Storage recovery/account-isolation tests. Existing unrelated WASM
warnings remain. The production GUI also needs its normal Pages build;
Windows, macOS and the remaining CI jobs remain the PR's checks. No merge
is performed as part of this investigation.

## Avatar comparison workflow instrumentation

Changing an avatar file activates the existing `Avatar restore` comparison
workflow. Its base step exposed another baseline harness problem on #290:
E0428, duplicate `probe` module and duplicate `PERSISTENT_READS` static.
The workflow unconditionally injected instrumentation that #282 had already
committed into main. It also would have double-counted reads by adding a
second increment. This failed while measuring the unchanged base, before
running the candidate.

The injection now adds each declaration/increment only when absent. Both
legacy uninstrumented inputs and current main were exercised through the
actual Python block extracted from the workflow. Each produced one probe,
one counter and one increment; applying the block twice was byte-identical.
For current main the entire output was byte-identical to the input. Both
base/candidate measurements and PNG/JPEG probe runs remain enabled. This is
an idempotency fix, with no deletion of the performance or lifecycle checks.
Loading