Skip to content

Session sync performance optimizations#63

Closed
Day-Anger wants to merge 17 commits into
iHildy:mainfrom
Day-Anger:pr/perf-optimizations
Closed

Session sync performance optimizations#63
Day-Anger wants to merge 17 commits into
iHildy:mainfrom
Day-Anger:pr/perf-optimizations

Conversation

@Day-Anger
Copy link
Copy Markdown

Summary

Five independent optimizations for the SQLite session synchronization, each verified with tests.

Optimizations

  1. Compact JSON — \JSON.stringify(session)\ without
    ull, 2. Saves ~30% file size on session JSON files.

  2. Single WAL checkpoint — checkpoint moved out of \openDB()\ into \syncSessions(), run once per cycle instead of on every DB open.

  3. Single DB handle — \syncSessions()\ opens SQLite once, uses handle-variant functions (\listSessionIdsFromHandle,
    eadSessionFromHandle, \writeSessionsToHandle) for all read/write operations within one cycle.

  4. Batch DB writes — all merged sessions written in a single \BEGIN…COMMIT\ transaction instead of one INSERT per session.

  5. Background git push — \pushBranch(...).catch(...)\ doesn't block startup. HEAD cache (\getHeadHash()\ + \state.lastHead) skips unnecessary remote fetches when HEAD hasn't changed.

Impact

Metrics from local testing (Windows, ~200 sessions):

  • Startup time: ~2.5s → ~0.8s
  • Sync cycle time: ~4s → ~1s
  • Disk usage: reduced by ~30% (compact JSON)

Day-Anger added 17 commits May 22, 2026 15:39
- Add notify() helper: emoji + toast + log in one call
- startupSync: 🔄 Syncing..., ✅ ready, ❌ error
- pull: 📥 Pulling..., 📥 result
- push: 📤 Pushing..., 📤 result
- link: 🔗 Linking..., 🔗 result
- init: 🚀 Sync configured
- session-db.ts: read/write sessions from SQLite and JSON dir
- session-merge.ts: syncSessions() — union-merge per record
  (messages, parts, todos, session_messages, shares by id+time_updated)
- service.ts: integrate syncSessions in pull, push, runStartup
- @types/node: ^22.15.0 for node:sqlite types
- Config field includeProjects (bool, default false)
- resolveProjectsFilePath(): Windows → %APPDATA%\ai.opencode.desktop\, Linux/macOS → /opencode
- SyncItem with preserveWhenMissing: true for graceful handling
- shell.ts: createNodeShell() wraps child_process.execSync with Bun-compatible tagged-template API
- service.ts: detect ctx.$ === undefined at createSyncService entry, replace with createNodeShell()
- Enables gh/git commands in OpenCode Desktop (Electron/Node.js)
…ned/objects

- All insert* functions now pass values through v() = asSQLValue()
- asSQLValue() now also serializes objects/arrays via JSON.stringify
- Fixes 'Provided value cannot be bound to SQLite parameter' on startup
projects-merge.ts: syncGlobalData() — union-merge проектов
  * server.projects.local — объединение по worktree
  * layout.page.lastProjectSession — объединение по директории, новее at
  * остальные ключи — если нет в локальном, берётся с удалённого
service.ts: вызов syncGlobalData в pull/push/startup
paths.ts: удалён старый SyncItem для простого копирования
При пустой директории remoteSessions (первый запуск, нет
удалённых сессий) syncSessions пишет только JSON-файлы,
не перезаписывая локальную БД. Это избегает ошибки
'Provided value cannot be bound to SQLite parameter 1',
которая возникала из-за NOT NULL constraint в таблицах
session/message/part при DELETE+INSERT
- Убирает deleteSession + каскадное удаление
- Все insert* функции заменены на upsert* (INSERT OR REPLACE)
- Избегает NOT NULL constraint violations при перезаписи сессий
- Предотвращает ошибку 'Provided value cannot be bound to SQLite parameter 1'
- Убрано копирование opencode.db и storage/* (SESSION_DIRS)
- Сессии синхронизируются только через per-session JSON merge (syncSessions)
- Предотвращает затирание локальных сессий при pull
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the synchronization service by replacing legacy bash scripts with a more integrated startup sync process that handles auto-committing, background pushing, and merging of session and project data. It also introduces a new SQLite-based session database and a Node-based shell wrapper. Feedback highlights a synchronization logic flaw where the 'shouldFetch' optimization might miss remote updates from other devices. Additionally, reviewers pointed out a race condition in the background push state updates and performance issues in the database layer, specifically N+1 queries and inefficient SQL statement preparation.

Comment thread src/sync/service.ts
});
await showToast(ctx.client, 'Config updated. Restart opencode to apply.', 'info');
return;
const shouldFetch = !head || head !== state.lastHead || dirty;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The 'shouldFetch' optimization breaks multi-device synchronization. It assumes that if the local repository is clean and the local HEAD matches the cached 'state.lastHead', no remote changes exist. However, another device could have pushed changes to the remote repository in the meantime. Without a 'fetch' (or at least an 'ls-remote' check), this client will not detect those changes on startup. Correctness should be prioritized over skipping a network request in a sync service. Consider using 'git ls-remote' if you want a faster check than a full fetch.

Comment thread src/sync/service.ts
Comment on lines +1447 to +1455
pushBranch(ctx.$, repoRoot, branch).catch((err) =>
log.warn('Background push failed', { error: formatError(err) })
);
await updateState(locations, {
lastPush: new Date().toISOString(),
lastHead: commitHead ?? undefined,
});
log.info('Auto-committed and pushed pending changes');
await showToast(ctx.client, 'Pending changes committed and pushed', 'info');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Updating the sync state ('lastPush', 'lastHead') and notifying the user immediately after starting a background push is unsafe. If the push fails (e.g., due to network issues or remote rejection), the local state will incorrectly reflect a successful synchronization. This, combined with the 'shouldFetch' optimization, will cause the client to skip future sync attempts for these changes. The state update and success notification should be moved inside the completion handler of the push operation.

      pushBranch(ctx.$, repoRoot, branch)
        .then(async () => {
          await updateState(locations, {
            lastPush: new Date().toISOString(),
            lastHead: commitHead ?? undefined,
          });
          log.info('Auto-committed and pushed pending changes');
          await showToast(ctx.client, 'Pending changes committed and pushed', 'info');
        })
        .catch((err) =>
          log.warn('Background push failed', { error: formatError(err) })
        );

Comment thread src/sync/session-db.ts
Comment on lines +166 to +178
function readMessages(db: DatabaseSync, sessionId: string): Message[] {
const rows = db
.prepare(`SELECT ${MESSAGE_COLUMNS.join(', ')} FROM message WHERE session_id = ?`)
.all(sessionId) as Record<string, unknown>[];
return rows.map((row) => {
const msg = row as unknown as Message;
const partRows = db
.prepare(`SELECT ${PART_COLUMNS.join(', ')} FROM part WHERE message_id = ?`)
.all(msg.id) as Record<string, unknown>[];
msg.parts = partRows.map((pr) => pr as unknown as Part);
return msg;
});
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This function has an N+1 query problem and redundant statement preparation. It prepares and executes a query for the 'part' table for every single message in the session. It is significantly more efficient to fetch all parts for the session in one query and group them by 'message_id' in memory.

function readMessages(db: DatabaseSync, sessionId: string): Message[] {
  const messages = db
    .prepare('SELECT ' + MESSAGE_COLUMNS.join(', ') + ' FROM message WHERE session_id = ?')
    .all(sessionId) as unknown as Message[];
  const allParts = db
    .prepare('SELECT ' + PART_COLUMNS.join(', ') + ' FROM part WHERE session_id = ?')
    .all(sessionId) as unknown as Part[];

  const partsByMessage = new Map<string, Part[]>();
  for (const part of allParts) {
    const list = partsByMessage.get(part.message_id) ?? [];
    list.push(part);
    partsByMessage.set(part.message_id, list);
  }

  for (const msg of messages) {
    msg.parts = partsByMessage.get(msg.id) ?? [];
  }
  return messages;
}

Comment thread src/sync/session-db.ts
Comment on lines +378 to +380
db.prepare(
`INSERT OR REPLACE INTO session (${SESSION_COLUMNS.join(', ')}) VALUES (${SESSION_COLUMNS.map(() => '?').join(',')})`
).run(...values);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Preparing the SQL statement inside the loop (via 'upsertSession' called in 'writeSessionsToHandle') is inefficient. The statement should be prepared once outside the loop and reused for all sessions to improve performance.

@jules-relay
Copy link
Copy Markdown

jules-relay Bot commented May 23, 2026

🤖 Review Jules Relay

I found 1 Gemini suggestion so far.

Type /relay batch to send all suggestions to Jules.

@Day-Anger Day-Anger closed this May 25, 2026
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