Skip to content

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

Merged
Shougo merged 2 commits intomainfrom
copilot/apply-changes-from-pr-39
Apr 5, 2026
Merged

loader: make registerPath concurrency-safe and add registerPaths (port from dpp.vim #39)#4
Shougo merged 2 commits intomainfrom
copilot/apply-changes-from-pr-39

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 5, 2026

Ports dpp.vim PR #39 to ddt.vim: registerPath held the lock across the full Deno.stat + importPlugin call, serializing all UI extension loads. This refactor moves I/O outside the lock so concurrent calls overlap.

denops/ddt/loader.ts

  • registerPath: fast-path returns early if already registered; Deno.stat + importPlugin run outside the lock; lock acquired only for the state-update critical section with an inner re-check to prevent duplicate registration from concurrent callers that raced past the fast-path
  • Removed private #register and the Mod wrapper type — logic inlined into registerPath
  • registerPaths(type, paths): new public method — fans out to registerPath via Promise.allSettled; logs per-path failures via console.error without throwing
// Before: stat + import serialized under lock
await this.#registerLock.lock(async () => {
  await this.#register(type, path); // blocks all concurrent callers
});

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

Tests

  • denops/ddt/tests/loader_concurrent_test.ts: covers single registration, concurrent same-path idempotency (8 parallel calls), concurrent different paths, non-existent path rejection, registerPaths with mixed valid/invalid paths, overlapping concurrent registerPaths, re-registration no-op
  • denops/ddt/tests/testdata/ui_{alpha,beta,gamma}.ts: minimal fixture files exporting a Ui class (adapted from dpp.vim's Ext fixtures to match ddt.vim's "ui" type)
  • denops/ddt/deno.json: added @std/assert@~1.0.7 import required by the new 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 #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 リンクを順次こちらに報告します。

User: @copilot Accepted Confirmation: Confirm agent session

Assistant: 承知しました — エージェントセッション確認済みです。これから実作業を開始します。進める内容を簡潔にまとめます。

やること(各リポジトリ: Shougo/ddc.vim, Shougo/ddu.vim, Shougo/ddx.vim, Shougo/ddt.vim)

  • ブランチを作成して denops/*/loader.ts を修正
    • fast-path: 既に登録済みなら即 return
    • 実ファイル I/O / import はロックの外で実行
    • import 成功後にロック下で #exts / #checkPaths 等の状態を更新し、重複登録を再チェック
    • Deno モジュールキャッシュ問題を検出したら dpp.vim と同様の console.warn を出力
    • registerPaths(paths: string[]) を追加(複数を並列で registerPath に投げる。失敗は console.error で記録)
    • 既存公開 API の挙動は壊さないよう調整
  • テスト追加
    • denops/*/tests/loader_concurrent_test.ts(dpp.vim のテストに相当)を追加
    • denops/*/tests/testdata に ext_alpha / ext_beta / ext_gamma の minimal fixture を追加
    • 指定のユースケース(同時登録・異常系・再登録など)を検証するテスト群を追加
  • 型名・パスは各リポジトリの現状に合わせて微調整
  • コミット → プッシュ → main への PR を作成
    • PR タイトル: loader: make registerPath concurrency-safe and add regist...

This pull request was created from Copilot chat.

Copilot AI changed the title [WIP] Apply changes from PR #39 to multiple repositories loader: make registerPath concurrency-safe and add registerPaths (port from dpp.vim #39) Apr 5, 2026
Copilot AI requested a review from Shougo April 5, 2026 03:48
@Shougo Shougo marked this pull request as ready for review April 5, 2026 03:49
@Shougo Shougo merged commit ba14833 into main Apr 5, 2026
3 checks passed
@Shougo Shougo deleted the copilot/apply-changes-from-pr-39 branch April 5, 2026 03:52
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