Skip to content

loader: make registerPath concurrency-safe and add registerPaths (from dpp.vim PR #39)#172

Merged
Shougo merged 5 commits intomainfrom
copilot/update-loader-ts-for-fast-path
Apr 5, 2026
Merged

loader: make registerPath concurrency-safe and add registerPaths (from dpp.vim PR #39)#172
Shougo merged 5 commits intomainfrom
copilot/update-loader-ts-for-fast-path

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 4, 2026

Ports the concurrency fixes from dpp.vim PR #39 to ddc.vim, adapted for ddc's three extension types (ui, source, filter) and its alias system.

denops/ddc/loader.ts

  • registerPath rewrite: I/O (Deno.stat + importPlugin) now runs outside the lock, enabling concurrent registrations of different paths to proceed in parallel. Shared state (#exts, #checkPaths) is updated inside the lock with a double-checked re-entry guard.
  • Fast-path: early return if path is already in #checkPaths, skipping all I/O.
  • Alias handling preserved inside the lock (ddc-specific, absent from dpp.vim).
  • registerPaths(type, paths) added: fans out to registerPath via Promise.allSettled, absorbing per-path errors with console.error rather than throwing.
  • Removed the now-redundant #register private method and the Mod type.
// Before: entire operation (I/O + state update) serialised under one lock
await this.#registerLock.lock(async () => {
  await this.#register(type, path);  // Deno.stat + import + state update
});

// After: I/O in parallel, only state mutation under lock
const importedMod = await importPlugin(entryPoint);   // outside lock
await this.#registerLock.lock(() => {
  if (path in this.#checkPaths) return;               // re-check inside
  // … update #exts and #checkPaths …
});

Tests (denops/ddc/tests/)

  • loader_concurrent_test.ts: covers single-path registration, concurrent same-path idempotency, concurrent different-path registration, non-existent path rejection, registerPaths with mixed valid/invalid paths, and re-registration no-op.
  • tests/testdata/source_{alpha,beta,gamma}.ts: minimal Source class fixtures used by the tests.
Original prompt

以下の変更を dpp.vim の PR #39 (Shougo/dpp.vim#39) と同様に各リポジトリへ適用してください。

対象リポジトリ: Shougo/ddc.vim, Shougo/ddu.vim, Shougo/ddx.vim, Shougo/ddt.vim

変更内容(実装・テスト両方):

  1. denops/*/loader.ts の改修

    • 既存の private helper (#register 相当) を置き換え、registerPath を以下の振る舞いにする:
      • 早期パス(fast-path): path が既に登録済みなら即座に return する。
      • 実際の I/O と import はロックの外で行う(同時実行される別スレッドと並列に処理させる)。
      • import 成功後に内部状態(#exts と #checkPaths 等)を更新する処理のみをロック下で行い、同時に他の呼び出しが同じ path を登録していないか再チェックする。
      • import 時に Deno のモジュールキャッシュ問題が検出された場合は dpp.vim の実装と同じ警告(console.warn によるメッセージ)を出す。
    • registerPaths(paths: string[]) を追加し、複数パスを並列で registerPath に渡して登録する。失敗したパスは console.error に出力しつつ他の登録は続行する。
    • 既存の公開 API(registerExtension, getExtension 等)の外部挙動は変えない。
  2. テスト追加

    • denops/*/tests ディレクトリに dpp.vim と同様のテストファイル(loader_concurrent_test.ts 相当)を追加し、以下を検証するテストを含める:
      • registerPath: 単一パスの登録が成功すること
      • registerPath: 同一パスの同時登録が冪等(いずれも成功しエラーにならない)であること
      • registerPath: 異なる複数パスの同時登録が成功すること
      • registerPath: 存在しないパスを登録すると例外が投げられること
      • registerPaths: 複数パス登録が成功すること、かつ一部無効なパスがあっても全体で例外を投げないこと
      • registerPaths の同時呼び出しが冪等であること
      • 再登録(成功後の再呼び出し)は no-op であること
      • テスト用フィクスチャ(testdata/*.ts)が読み込み可能であること
    • dpp.vim と同様に簡易の fixture モジュール(Ext/Protocol の minimal クラス)を denops/*/tests/testdata に追加する。
  3. 既存のコードスタイル・型・パスに合わせて微調整

    • 各リポジトリの loader.ts の実際の名前空間・型定義(Ext/Protocol のクラス名や DppExtType 相当の命名)は各プロジェクトに合わせて調整してください。
    • 既存の importPlugin / isDenoCacheIssueError / EXT_ENTRY_POINT_FILE / Lock 等のユーティリティが各リポジトリに存在する想定です。見つからない場合は同等の処理を持つファイルを探して適切に参照すること。
  4. PR 作成に関するメタデータ

    • PR タイトル: "loader: make registerPath concurrency-safe and add registerPaths + tests (from dpp.vim PR msgpack TimeoutErrors #39)"
    • PR 説明: dpp.vim の PR msgpack TimeoutErrors #39 への参照を含め、上記の変更点を要約して記載してください。
    • ベースブランチ: main

重要な注意点:

  • dpp.vim の変更をそのままコピペするのではなく、各リポジトリの既存 API と型に合わせて差し戻しがないように実装してください。
  • 単体テストは Deno の環境で実行できることを想定し、package/import の相対パスや URL を正しく解決してください。

もし実装上の差異や追加で確認が必要な点が見つかった場合は、PR の説明にその差分と理由を明記し、該当箇所でコメントしてください。

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Shougo/dpp.vim#39

これと同様の変更を ddc.vim, ddu.vim, ddx.vim, ddt.vim に適用してください。

Assistant: 了解しました — これらのリポジトリに dpp.vim の PR #39 と同様の変更を適用して PR を作成できますが、いくつか確認させてください。

確認事項(短く答えてください):

  1. リポジトリのオーナーは「Shougo」でよいですか?(対象は Shougo/ddc.vim, Shougo/ddu.vim, Shougo/ddx.vim, Shougo/ddt.vim で良いか)
  2. 各リポジトリで PR を直接作成してよいですか?(はい → こちらでブランチ作成・コミット・PR を作ります)
  3. PR のベースブランチは main(または master)でよいですか?(指定がなければ main を想定します)
  4. PR タイトル/説明は自動で「Apply loader concurrency and registerPaths changes (from dpp.vim PR msgpack TimeoutErrors #39)」のように作成して良いですか。別文面が良ければ教えてください。

上の確認をいただければ、承諾後に各リポジトリへ同様の変更を適用して PR を作成します。

User: はい

Assistant: 先ほどのツール呼び出しのダイアログで承認をお願いします — 承認を確認次第、各リポジトリへの変更適用とPR作成を開始します。

This pull request was created from Copilot chat.

Copilot AI changed the title [WIP] Update denops loader.ts for improved path registration loader: make registerPath concurrency-safe and add registerPaths (from dpp.vim PR #39) Apr 4, 2026
Copilot AI requested a review from Shougo April 4, 2026 06:39
@Shougo Shougo marked this pull request as ready for review April 5, 2026 03:22
@Shougo Shougo merged commit b1eff14 into main Apr 5, 2026
3 checks passed
@Shougo Shougo deleted the copilot/update-loader-ts-for-fast-path branch April 5, 2026 03:24
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.

2 participants