Skip to content

perf(sync): reduce repeated parent cleanup during updates - #600

Merged
christso merged 1 commit into
mainfrom
perf/remaining-cli-performance
Oct 28, 2026
Merged

christso merged 1 commit into
mainfrom
perf/remaining-cli-performance

Conversation

@christso

@christso christso commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Repeated workspace updates finish faster for large skill trees by cleaning each distinct parent after its tracked sibling deletions finish. The measured 1,000-skill medians fell 12.5–16.5%, with existing ownership and symlink checks retained and immediate cleanup preserved for overlapping state entries.

The baseline is e331f44 (1.21.4). Existing unchanged-remote skips are included in that baseline; this change reduces remaining local filesystem work. Startup and copying remain substantial costs.

Measurements

Three interleaved warmups per build, then 15 timed baseline/candidate pairs per scenario, alternating order. These are warm-filesystem measurements with isolated HOME/project fixtures, one Codex file-install client, and local bare Git remotes. There is no injected latency or live-network speedup claim. With 15 samples, nearest-rank p95 is the maximum sample, not a stable production-tail estimate.

1,000-skill update Baseline p50 / p95 (ms) Candidate p50 / p95 (ms)
Local project 1490 / 1647 1269 / 1377
Local user 1448 / 1548 1268 / 1388
Unchanged remote project 1609 / 1706 1373 / 1476
Unchanged remote user 1620 / 1818 1352 / 1462
Offline cached project 1493 / 1558 1294 / 1349
Offline cached user 1506 / 1634 1288 / 1361

The separate local-project trace reduced rmdir calls from 3,000 to 2,001, readlink from 31,018 to 21,028, statx from 28,032 to 26,071, and access from 4,020 to 3,021. Git work stayed identical: unchanged online updates made one ls-remote and zero clone/pull/fetch/merge/checkout/reset operations; offline updates made no remote calls. Small startup/empty-update timing differences are treated as noise.

Environment: Node v26.7.0, Bun 1.4.3, Linux 7.0.0-38-generic, Intel i7-6700T. The report includes all 0/100/1,000-skill scenarios and the exact baseline/candidate bundle hashes.

Related: #594, already included in the baseline; its remote skip receives no performance credit in this PR.

Validation

  • 95 focused tests passed, with 803 assertions, covering shared/nested cleanup, retained user files, write-root preservation, final symlinks, refused symlink-parent traversal, external Pi roots, and overlapping state reporting.
  • bun run typecheck, explicit benchmark-script typecheck, bun run lint, and bun run build passed. Lint retains the existing 17 warnings.
  • All 360 timed invocations matched stdout/stderr, destination contents (including canonical .agents), normalized state, and Git counts. All 450 invocations, including initial materialization, warmups and traces, preserved expected source/cached-resource, config, and sentinel bytes/modes.
  • A one-shot candidate wrapper deleted the HOME sentinel on its first invocation; the protected-state guard rejected it during the first warmup with exit 1.
  • Built baseline/candidate lifecycle checks installed, repeated updates, edited/removed source skills, and removed the plugin in project and user scope. All eight normalized JSON/redirected-human outputs matched. Source edits reached destinations, removed skills disappeared, user-owned sentinel bytes survived, and final state released owned files.
  • Built JSON update --scope project --offline --dry-run and status preserved every fixture path, byte, mode, mtime, and symlink target after materialization.
  • Independent peer review found no actionable production issue. Measurement findings about canonical-resource coverage and first-warmup contamination were resolved; the published samples were collected with the final protected-state guards.

Focused command:

bun test tests/unit/core/sync-dedup.test.ts tests/unit/core/sync.test.ts \
  tests/unit/core/sync-user.test.ts tests/unit/core/client-context.test.ts \
  tests/unit/core/sync-symlink.test.ts tests/unit/core/omp-artifacts.test.ts

Exact measured command, using the immutable baseline copied before edits:

bun scripts/benchmark-cli-sync.ts \
  /tmp/allagents-perf-e331f44/baseline/index.js dist/index.js \
  --samples=15 --warmups=3 --sizes=0,100,1000 --trace

The report also provides commands to build baseline e331f44 in an isolated checkout from an already-modified branch. Broad integration validation is being run separately; no duplicate full suite was launched for publication.

Evidence

The raw evidence contains task paths and benchmark metadata, with no real-user configuration or credentials. No live-network, cold-disk-cache, failed-remote, cancellation-latency, or multi-profile improvement is established.

Combined batch verification

The parent integrated #598, #599 and #600 on clean baseline e331f44 (integration tree 54a8c5338bfa1caf0a187bae4c8cc94895338352) without conflicts. The integration tree is equivalent to applying these three PRs in that order.

  • bun test: 2,402 passed, 7 skipped, 0 failed, 9,742 assertions across 200 files (171.53s).
  • bun run build, bun run typecheck, bun run lint, bun run schema:check, bun run docs:build, bun run package:check and bun run package:smoke: passed. Combined lint has zero warnings. Package smoke checked 54 help nodes.
  • Manual built-Node CLI checks used isolated HOME/project/plugin fixtures. Valid and invalid validation preserved bytes and mtimes; root skills with supporting prompts and unnamed sidecar packages validated; repeated updates copied resources without executing scripts; user-owned sentinels survived update/removal; an MCP scalar edit preserved BOM, CRLF, comments and flow formatting; uninstall removed owned skills. Temporary fixtures were removed.
  • A bounded combined-build check used python3 /tmp/allagents-remaining-integration-manual.py (script reproduced in perf(sync): reduce repeated parent cleanup during updates #600) with five alternating baseline/candidate pairs after one materialization run per build. Its guards checked output/source/config/sentinel/state and client destination compatibility. Local-project median was 1,352.27ms → 1,183.30ms (12.49% lower). This is a corroborating warm-cache measurement; perf(sync): reduce repeated parent cleanup during updates #600's larger paired matrix is the primary performance evidence.
  • The first full local run hit four timeouts in existing skill-install/update tests. Both affected files then passed unchanged (27 tests), followed by the clean full-suite result above. No timeout limits or production code were changed between those runs, and the initial cause is not established.
  • All seven GitHub Actions CI jobs also passed separately on each PR's exact head, including Windows command argv and Node 24 smoke. Independent review findings are resolved.
Combined manual CLI and bounded performance reproduction

Build the combined candidate and baseline e331f44 as described in the performance report, then set candidate and baseline to those bundle paths. Save the script as /tmp/allagents-remaining-integration-manual.py and run python3 /tmp/allagents-remaining-integration-manual.py. The script removes its temporary fixtures.

import hashlib
import json
import os
from pathlib import Path
import shutil
import statistics
import subprocess
import tempfile
import time

candidate = Path('/home/christso/projects/allagentsdev/allagents.worktrees/test-remaining-work-integration/dist/index.js')
baseline = Path('/tmp/allagents-perf-e331f44/baseline/index.js')

def fingerprint(root, timestamps=False):
    entries = []
    if not root.exists():
        return []
    for path in sorted(root.rglob('*')):
        stats = path.lstat()
        item = [str(path.relative_to(root)), stats.st_mode & 0o777]
        if timestamps:
            item.append(stats.st_mtime_ns)
        if path.is_symlink():
            item.extend(['link', os.readlink(path)])
        elif path.is_file():
            item.extend(['file', hashlib.sha256(path.read_bytes()).hexdigest()])
        else:
            item.append('directory')
        entries.append(item)
    return entries

root = Path(tempfile.mkdtemp(prefix='allagents-remaining-manual-'))
try:
    home, project, source = (root / name for name in ('home', 'project', 'source'))
    for path in (home, project, source, project / '.allagents', source / 'scripts', source / 'prompts'):
        path.mkdir(parents=True, exist_ok=True)
    (source / 'SKILL.md').write_text('---\nname: demo\ndescription: Combined verification\n---\nRead prompts/review.md and scripts/hello.sh.\n')
    (source / 'scripts/hello.sh').write_text('echo supported\n')
    (source / 'prompts/review.md').write_text('Supporting material\n')
    (source / 'package.json').write_text('{"private":true,"scripts":{"prepare":"touch executed"}}')
    config = project / '.allagents/workspace.yaml'
    original = ('\ufeff# Keep this header\r\nversion: 2\r\nrepositories: []\r\nclients: [copilot]\r\nsyncMode: copy\r\nplugins:\r\n  - source: ' + json.dumps(str(source)) + '\r\n    install: file\r\nmcpServers:\r\n    runtime:\r\n        type: stdio\r\n        command: node\r\n        args: [keep.js, --flag]\r\n# Keep this footer without final newline')
    config.write_bytes(original.encode())
    sentinel = project / '.github/skills/personal/keep.txt'
    sentinel.parent.mkdir(parents=True)
    sentinel.write_text('user owned')
    env = {**os.environ, 'HOME': str(home), 'USERPROFILE': str(home), 'ALLAGENTS_TEST_HOME': str(home), 'CI': '1'}
    def run(*args, expected=0):
        result = subprocess.run(['node', str(candidate), '--json', *args], cwd=project, env=env, capture_output=True, text=True, timeout=60)
        assert result.returncode == expected, result.stdout + result.stderr
        payload = json.loads(result.stdout)
        assert payload['success'] == (expected == 0)
        assert result.stderr == '', result.stderr
        return payload
    before = fingerprint(root, timestamps=True)
    valid = run('plugin', 'validate', str(source))
    assert valid['data']['valid'] is True and valid['data']['validation']['skills'] == 1
    assert fingerprint(root, timestamps=True) == before
    invalid = run('plugin', 'validate', str(root / 'missing'), expected=1)
    assert invalid['data']['valid'] is False
    assert fingerprint(root, timestamps=True) == before
    for _ in range(2):
        run('update', '--scope', 'project', '--offline')
        assert (project / '.github/skills/demo/scripts/hello.sh').read_text() == 'echo supported\n'
        assert sentinel.read_text() == 'user owned'
        assert config.read_bytes() == original.encode()
    run('mcp', 'add', 'runtime', 'bun', '--arg=keep.js', '--arg=--flag', '--scope', 'project', '--force')
    assert config.read_bytes() == original.replace('command: node', 'command: bun').encode()
    run('plugin', 'uninstall', str(source), '--scope', 'project', '--yes')
    assert sentinel.read_text() == 'user owned'
    assert not (project / '.github/skills/demo').exists()
    assert not list(root.rglob('executed'))
    print(json.dumps({'combined_manual': 'passed', 'checks': ['valid/invalid plugin validation is read-only including mtimes', 'root skill sidecar compatibility', 'repeat update and supporting files', 'user sentinel preservation', 'MCP edit preserves exact unrelated BOM/CRLF/comment/flow bytes', 'owned skill cleanup', 'plugin scripts never executed'], 'fixture_cleanup': 'removed'}), flush=True)
finally:
    shutil.rmtree(root)

root = Path(tempfile.mkdtemp(prefix='allagents-remaining-confirm-'))
try:
    home, project, source = (root / name for name in ('home', 'project', 'source'))
    for path in (home, project / '.allagents', source):
        path.mkdir(parents=True, exist_ok=True)
    for index in range(1000):
        path = source / 'skills' / ('skill-' + str(index))
        path.mkdir(parents=True)
        (path / 'SKILL.md').write_text('---\nname: skill-' + str(index) + '\ndescription: fixture\n---\n# Skill\n')
    config = project / '.allagents/workspace.yaml'
    original = 'version: 2\nrepositories: []\nclients: [codex]\nsyncMode: copy\nplugins:\n  - source: ' + json.dumps(str(source)) + '\n    install: file\n'
    config.write_text(original)
    sentinel = project / '.codex/skills/user-owned/keep.txt'
    sentinel.parent.mkdir(parents=True)
    sentinel.write_text('preserve')
    expected_source = fingerprint(source)
    env = {**os.environ, 'HOME': str(home), 'USERPROFILE': str(home), 'ALLAGENTS_TEST_HOME': str(home), 'CI': '1', 'GIT_CONFIG_GLOBAL': '/dev/null', 'GIT_CONFIG_NOSYSTEM': '1'}
    samples = {'baseline': [], 'combined': []}
    signatures = set()
    def run(label, timed=False):
        cli = baseline if label == 'baseline' else candidate
        started = time.perf_counter()
        result = subprocess.run(['node', str(cli), '--json', 'update', '--scope', 'project', '--offline'], cwd=project, env=env, capture_output=True, text=True, timeout=60)
        elapsed = (time.perf_counter() - started) * 1000
        assert result.returncode == 0, result.stdout + result.stderr
        payload = json.loads(result.stdout)
        assert payload['success'] is True
        assert fingerprint(source) == expected_source
        assert config.read_text() == original and sentinel.read_text() == 'preserve'
        state = json.loads((project / '.allagents/sync-state.json').read_text())
        state.pop('lastSync', None)
        signature = json.dumps([payload, state, fingerprint(project / '.codex')], sort_keys=True)
        if timed:
            signatures.add(signature)
            samples[label].append(elapsed)
    run('baseline')
    run('combined')
    for index in range(5):
        for label in (('baseline', 'combined') if index % 2 == 0 else ('combined', 'baseline')):
            run(label, timed=True)
    assert len(signatures) == 1
    before = statistics.median(samples['baseline'])
    after = statistics.median(samples['combined'])
    print(json.dumps({'combined_performance_confirmation': 'passed', 'scenario': '1000-skill local project offline, copy mode', 'pairs': 5, 'baseline': 'e331f44', 'baseline_p50_ms': round(before, 2), 'combined_p50_ms': round(after, 2), 'median_reduction_percent': round((before-after)/before*100, 2), 'samples_ms': samples, 'guards': ['source content/modes', 'workspace config', 'sentinel', 'normalized output/state/destination compatibility'], 'fixture_cleanup': 'removed'}), flush=True)
finally:
    shutil.rmtree(root)

@christso
christso merged commit 3c29ad4 into main Oct 28, 2026
8 checks passed
@christso
christso deleted the perf/remaining-cli-performance branch October 28, 2026 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant