Skip to content

refactor(loader): parallelize I/O in registerPath; add registerPaths#39

Merged
Shougo merged 3 commits intomainfrom
copilot/parallelize-loader-module
Apr 4, 2026
Merged

refactor(loader): parallelize I/O in registerPath; add registerPaths#39
Shougo merged 3 commits intomainfrom
copilot/parallelize-loader-module

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 2, 2026

registerPath held the register lock across the full Deno.stat + importPlugin call, serializing all extension loads. With many extensions this compounds into significant startup latency.

Changes

denops/dpp/loader.ts

  • registerPath: I/O (Deno.stat + importPlugin) moved outside the lock so concurrent calls overlap. Lock is acquired only for the state-update critical section. Re-checks #checkPaths inside the lock to prevent duplicate registration when concurrent callers race past the fast-path.
  • Removed Mod wrapper type: importedMod is now a local any variable, initialized only after successful I/O.
  • #register: Kept; now delegates to registerPath.
  • registerPaths(type, paths): New public method — fans out to registerPath via Promise.allSettled; logs failures per-path without throwing.
// Before: each registerPath call blocked on I/O under the lock
await this.#registerLock.lock(async () => {
  await this.#register(type, path); // stat + import inside lock
});

// After: I/O runs concurrently; 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
});

denops/dpp/tests/loader_concurrent_test.ts

New tests covering: single registration, concurrent same-path (idempotency, 8 parallel calls), concurrent different-paths, invalid-path error propagation, registerPaths with mixed valid/invalid paths, and overlapping concurrent registerPaths calls.

Original prompt

Goal

Parallelize Loader module loading to reduce startup/registration latency when many extensions are present. Implemented changes should keep shared-state updates (this.#exts, this.#checkPaths) protected by the existing register lock to avoid races and double registration.

What to change (detailed)

  1. denops/dpp/loader.ts
  • Modify registerPath to perform heavy I/O (Deno.stat and importPlugin) outside the register lock so that multiple concurrent registerPath calls can perform I/O concurrently.
  • After the I/O completes, acquire the existing #registerLock for a short critical section to update shared state. Re-check this.#checkPaths in the critical section to avoid duplicate registration.
  • Keep the previous logging and error handling behavior, including Deno cache issue warnings.
  • Keep the existing private #register method for backward compatibility by delegating to the new registerPath implementation.
  • Add a new method registerPaths(type: DppExtType, paths: string[]): Promise that calls registerPath for multiple paths in parallel using Promise.allSettled. Log rejected entries but do not throw a consolidated error (this can be changed if requested).
  1. Tests
  • Add a test denops/dpp/tests/loader_concurrent_test.ts that verifies concurrent registration behavior:
    • Create small temporary module files (or use test fixtures) and call registerPath concurrently for the same and different paths.
    • Ensure shared state (checkPaths and exts) reflect unique registration (no duplicates) and there are no unhandled rejections.
  • Run the existing test suite to ensure regressions don't occur.

Commit strategy

  • Branch: refactor/loader-parallel-load
  • Commits:
    1. refactor(loader): perform stat/import outside register lock
    2. feat(loader): add registerPaths for concurrent registration
    3. test(loader): add concurrent registration test

Testing steps

  • Run deno tests: deno test -A --doc --parallel --shuffle denops/**/*.ts
  • Run new loader_concurrent_test.ts ensuring success in both single-run and parallel-run modes.

Notes and rationale

  • Shared mutations remain inside the lock to avoid races. The I/O is moved outside the lock to allow parallel imports and reduce total elapsed time when many extensions are loaded.
  • The final registration order may differ from strict sequential registration; however, because registration (state update) remains locked and re-checks exist, duplication is avoided. For extensions that require strict ordering, callers can still perform sequential registration.
  • The API surface remains backward compatible.

Please create the branch, commit the changes, run tests, and open a PR against main with an explanatory description. After creating the PR, provide the PR URL and summary of CI/test results. If any test failures occur, include the failing logs.

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

Assistant: 直接のおすすめ:まずは安全かつ影響範囲の小さい最適化から着手します。優先順は

  1. 不要な return await の削除(低リスク・一括適用可能)
  2. import-map 読み込みと ImportMapImporter のキャッシュ化(I/O削減)
    これらを実装してテストを実行し、問題なければ次に大きめのリファクタ(プロトコルキャッシュや I/O 最適化)に進みます。続けて PR を作成してよければ「PR作成して」と一言ください。すぐに作業します。

何を確認・実行済みか

  • あなたがテスト追加を手でマージしたことを確認しました(ありがとうございます)。これでリグレッションを避けやすくなっています。
  • リポジトリ内の TypeScript/deno 実装(denops/dpp/*)を確認しました。改善候補がいくつか見つかっています(下に詳細)。

安全で効果の高い具体的な最適化案(短期)

  • 変更案 1 — 不要な return await の削除(低リスク)

    • 影響:多数の async 関数で return await foo() が使われています。多くの場合 return foo() の方が余計な microtask を減らせてパフォーマンスとスタックトレースの扱いを改善できます(try/catch でスタックをホールドしたい場合は await を残す必要あり)。
    • 対象例(ファイル内で多数見つかりました):
      • denops/dpp/dpp.ts の extAction/getExt/getProtocols など
      • denops/dpp/app.ts の dispatcher 内の getExt/getProtocols 等
    • 実装方針:try/catch の中で例外のスタックを保持したい箇所はそのままにし、単純に結果を返すだけの箇所は return foo() に置換する。CI でテストを実行。
  • 変更案 2 — import-map 読み込みと ImportMapImporter のキャッシュ化(低〜中リスク)

    • 目的:同一ディレクトリにある複数の import を繰り返す場合に毎回ファイルを読んで処理しているので、import map のロードと作成した ImportMapImporter をディレクトリ単位でキャッシュして I/O を減らす。
    • 影響範囲:denops/dpp/utils.ts の tryLoadImportMap と importPlugin。動作は変えずにキャッシュを追加する実装を行う(キャッシュを使って import を呼ぶ)。
    • 実装方針(例):
      • module-scoped Map<string, ImportMap | null> を作成し、ディレクトリをキーに import map の有無/内容を保存。
      • ImportMapImporter も同様にキャッシュ(同一 import map を再利用)。
      • エッジケース:import_map ファイルの変更時は Deno キャッシュに依存しているため上書き検出は慎重に扱う(初版はプロセス内キャッシュのみにして、必要なら TTL や強制 reload タスクを追加)。

中期〜大きめのリファクタ案(要検証)

  • getProtocols の呼び出し回数削減/局所キャッシュ
    • makeState や extAction 内で同じオプションに対して複数回 getProtocols を呼ぶ場面があれば局所的にキャッシュして渡すことで重複ロードを削減できます。ただし Loader 側に既にキャッシュ(#exts)や autoload 処理があるため、効果はケースバイケース。まず計測→改善を推奨。
  • linkPath / ファイル操作の I/O 最適化
    • 現状は isDirectory / safeStat を何度も呼んでいるため、stat 結果の再利用やより効率的なファイルコピーロジックの導入を検討できます。リスクあるためユニットテストと手動検証が必要。
  • Record -> Map の置換(動的キーが多い箇所)
    • 多数のキー追加/削除/ルックアップがある箇所は Map にした方が速い場合があります。影響が大きいので効果測定後に段階的に置換。

具体的に私が変更するファイル(短期優先)

  • denops/dpp/dpp.ts — 不要な return await の簡潔化(影響小)
  • denops/dpp/app.ts — dispatcher での冗長 await の簡潔化
  • denops/dpp/utils.ts — import map キャッシュ追加、importPlugin の...

This pull request was created from Copilot chat.

… add concurrent tests

Agent-Logs-Url: https://github.com/Shougo/dpp.vim/sessions/7ed39eb1-3de6-465d-bd04-410eb88bf707

Co-authored-by: Shougo <41495+Shougo@users.noreply.github.com>
Copilot AI changed the title [WIP] Parallelize Loader module loading for reduced latency refactor(loader): parallelize I/O in registerPath; add registerPaths Apr 2, 2026
Copilot AI requested a review from Shougo April 2, 2026 04:26
@Shougo Shougo marked this pull request as ready for review April 4, 2026 06:17
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