Skip to content

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

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

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

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 5, 2026

Ports the concurrency improvements from dpp.vim PR #39 to ddx.vim, adapted for its DdxExtType = "ui" | "analyzer" type system.

loader.ts

  • registerPath: I/O (Deno.stat + importPlugin) now runs outside the lock so concurrent registrations of different paths proceed in parallel. State updates (#uis, #analyzers, #checkPaths) remain locked, with a re-check inside the lock to handle races. An unlocked fast-path at the top skips already-registered paths entirely.
  • registerPaths(type, paths): New public method — fans out to registerPath via Promise.allSettled, absorbs per-path errors with console.error, never throws.
  • Removed private #register helper and unused Mod type.
// Before: all I/O was serialized inside the lock
await this.#registerLock.lock(async () => {
  await this.#register(type, path); // stat + import + state update, all sequential
});

// After: I/O is parallel; only state mutation is locked
if (path in this.#checkPaths) return;          // fast-path (unlocked)
importedMod = await importPlugin(entryPoint);   // I/O outside lock
await this.#registerLock.lock(() => {           // state update only
  if (path in this.#checkPaths) return;         // re-check inside lock
  // ... register ui/analyzer ...
});

Tests

  • denops/ddx/tests/loader_concurrent_test.ts: covers single registration, concurrent same-path idempotency, concurrent different-path registration, non-existent path throws, registerPaths success, registerPaths partial-failure tolerance, concurrent registerPaths idempotency, re-registration no-op, and fixture URL round-trip.
  • denops/ddx/tests/testdata/ui_{alpha,beta,gamma}.ts: minimal Ui fixture classes for test use.
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 #39)"
    • PR 説明: dpp.vim の PR #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 #39)」のように作成して良いですか。別文面が良ければ教えてください。

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

User: はい

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

User: @copilot Accepted Confirmation: Confirm agent session

Assistant: 開始しました — ご依頼の変更適用作業を開始し、進捗は下のタスクで追跡できます。今から各リポジトリ(Shougo/ddc.vim, Shougo/ddu.vim, Shougo/ddx.vim, Shougo/ddt.vim)に対して以下を順に行います:

  • denops/*/loader.ts を dpp.vim の PR #39 と同等の設計に合わせて書き換え(並列 import → ロックで状態更新、fast-path、registerPaths の追加、エラーメッセージは同様に保持)
  • denops/*/tests に並列登録を検証するテストと testdata の fixture を追加
  • ブランチを作成→コミット→PR を作成(タイトル/説明はご指定のものを使用)
  • それぞれの PR リンクを作業完了ごとに報告

進めて問題があればここでお知らせください。追跡用タスクは以下です:

data:
    - title: '[WIP] Update denops loader.ts for improved path registration'
      repository: Shougo/ddc.vim
      url: https://github.com/copilot/tasks/pull/PR_kwDOFTvKk87P7dUW?session_id=bc7390b2-9739-4965-a027-447fefc6c1f2
      state: queued

次に、ddc.vim → ddu.vim → ddx.vim → ddt.vim の順で変更を適用して PR を作成します。PR 作成後に各 PR のリンクをここに貼ります。変更の内容や PR の本文で特に追記してほしい点があれば教えてください。

User: @copilot Accepted Confirmation: Confirm agent session

Assistant: 進捗報告です。状況を開始済みの作業に沿ってまとめます。

  • 既に ddc.vim と ddu.vim の作業をキューに入れ、変更適用と PR 作成の準備を開始しました(進捗タスクは先ほど共有済みです)。
  • 次に ddx.vim と ddt.vim を同様に処理して、各リポジトリに対して loader の並列安全化 + registerPaths の追加とテストを適用し、ブランチ作成→コミット→PR を行います。PR タイトル/説明はご指定どおりにします。

ただし、この応答では実際のリポジトリ変更・PR 作成ができないため(実行環境の制約)、ddx.vim と ddt.vim への変更を開始してよいか最終確認をお願いします。開始してよければ「開始して」でお知らせください。開始後は各リポジトリの PR リンクを順次こちらに報告します。

This pull request was created from Copilot chat.

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