Skip to content

perf(utils): optimize linkPath with safeStat consolidation and batched parallel directory traversal#40

Open
Copilot wants to merge 2 commits intomainfrom
copilot/refactor-linkpath-function
Open

perf(utils): optimize linkPath with safeStat consolidation and batched parallel directory traversal#40
Copilot wants to merge 2 commits intomainfrom
copilot/refactor-linkpath-function

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 5, 2026

linkPath made redundant stat/lstat calls per entry and processed directory contents sequentially, making it slow for large trees.

Changes

denops/dpp/utils.ts

  • LINK_BATCH_SIZE = 32 – controls concurrent I/O per directory level
  • linkPathInternal (private) – replaces the recursive body of linkPath:
    • Single safeStat(src) determines existence and type (eliminates the separate isDirectory(src) call which did its own Deno.stat)
    • Directory entries collected via Deno.readDir then processed in parallel batches with Promise.allSettled
    • File-level behavior unchanged (skip existing dest, Deno.link on Windows, Deno.symlink elsewhere)
  • linkPath public signature unchanged — delegates to linkPathInternal
// Before: sequential entry-by-entry recursion
for await (const entry of Deno.readDir(src)) {
  await linkPath(hasWindows, join(src, entry.name), join(dest, entry.name));
}

// After: collected then processed in batches of 32 concurrently
const entries: string[] = [];
for await (const entry of Deno.readDir(src)) entries.push(entry.name);
for (let i = 0; i < entries.length; i += LINK_BATCH_SIZE) {
  await Promise.allSettled(
    entries.slice(i, i + LINK_BATCH_SIZE).map((name) =>
      linkPathInternal(hasWindows, join(src, name), join(dest, name))
    ),
  );
}

denops/dpp/tests/linkpath_test.ts (new)

Tests covering (non-Windows): file symlinking, skip-existing-dest, recursive directory, nested subdirectories, no-op on missing src.

Original prompt

目的

  • linkPath 関数の I/O 性能を改善して多数ファイル/多数ディレクトリでの処理時間を短縮する。挙動は既存と互換性を保ちつつ、stat/lstat 呼び出しを最小化し、ディレクトリ再帰時の処理をバッチ並列化します。

変更内容(詳細)

  1. ファイル: denops/dpp/utils.ts
  • linkPath の実装を refactor して内部関数 linkPathInternal を導入。
    • linkPathInternal は safeStat を使って src の存在と型を確認。
    • ディレクトリの場合、dest を作成し、Deno.readDir でエントリ名を収集して、エントリをバッチ(デフォルト LINK_BATCH_SIZE = 32)で並列処理します(Promise.allSettled を使用)。
    • ファイルの場合、dest が既に存在すればスキップ。Windows では Deno.link、それ以外は Deno.symlink を用いる。
  • 既存の Windows / 非Windows 分岐、エラーハンドリング、safeStat/isDirectory の使用は維持。
  • 既存の API シグネチャは変更しない(export async function linkPath(hasWindows: boolean, src: string, dest: string))。
  1. 追加ファイル: denops/dpp/tests/linkpath_test.ts
  • linkPath の基本動作を検証するテストを追加(非Windows環境でファイル/ディレクトリが正しくリンクされることを検証)。
  • テストでは一時ディレクトリを作成し、サブディレクトリとファイルを作成して linkPath を実行し、安全に stat できることを assert します。

コミット方針

  • ブランチ: perf/linkpath-opt
  • コミット1: perf(utils): optimize linkPath to reduce stat calls and batch entries
  • コミット2: test(utils): add linkPath basic test

テスト手順(PR レビュー用)

  • deno test -A --doc --parallel --shuffle denops/**/*.ts
  • 追加テストは非Windows環境で実行されることを想定しています。Windows CI を通すにはハードリンク/シンボリック作成のパーミッションに注意してください。

PR 本文(レビュー向け要約)

  • 目的:多数ファイルや深いディレクトリ構造を処理する際の linkPath の待ち時間を減らす。
  • 変更点:stat 呼び出しを safeStat へ集約し、ディレクトリエントリをバッチ並列で処理することで全体の elapsed time を短縮。既存の振る舞い(既に存在するファイルはスキップ、Windows では hard link)を保持。
  • リスク:バッチ並列化により同時 I/O が増えるためスパイクが出る可能性があるが、デフォルトの LINK_BATCH_SIZE=32 で抑制している。必要に応じてバッチサイズを小さく設定できるよう拡張可能。

要求

  • 上記変更を perf/linkpath-opt ブランチにコミットし、既存テストと追加テストを実行して PR を作成してください。PR URL と CI 結果(テストログ)を返してください。テスト失敗があればログを添えてください。

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

User: パッチをまず確認しましょう

Assistant: 以下は Loader のモジュールロードを並列化するためのパッチ案です。要点は「重い I/O(Deno.stat / importPlugin)をロック外で実行し、共有状態への反映(this.#exts / this.#checkPaths)を短時間のクリティカルセクションで行う」ことです。並列で import を試みても最終的な登録処理はロック内で再チェックして一意化するため、二重登録の競合は回避します。

パッチ(denops/dpp/loader.ts の差分)
--- a/denops/dpp/loader.ts
+++ b/denops/dpp/loader.ts
@@ (省略して前後コンテキストを含めた抜粋、変更点に注目してください)

@@
   async registerPath(type: DppExtType, path: string): Promise<void> {
-    await this.#registerLock.lock(async () => {
-      try {
-        await this.#register(type, path);
-      } catch (e) {
-        if (isDenoCacheIssueError(e)) {
-          console.warn("*".repeat(80));
-          console.warn(`Deno module cache issue is detected.`);
-          console.warn(
-            `Execute '!deno cache --reload "${path}"' and restart Vim/Neovim.`,
-          );
-          console.warn("*".repeat(80));
-        }
-
-        console.error(`Failed to load file '${path}': ${e}`);
-        throw e;
-      }
-    });
+    // Fast-path: already registered.
+    if (path in this.#checkPaths) {
+      return;
+    }
+
+    // Perform file stat and module import outside of the register lock so that
+    // multiple registerPath calls can do I/O in parallel.
+    let fileInfo: Deno.FileInfo;
+    try {
+      // NOTE: We intentionally use Deno.stat here (keep same behavior).
+      fileInfo = await Deno.stat(path);
+    } catch (e) {
+      // Preserve original error handling semantics.
+      if (isDenoCacheIssueError(e)) {
+        console.warn("*".repeat(80));
+        console.warn(`Deno module cache issue is detected.`);
+        console.warn(
+          `Execute '!deno cache --reload "${path}"' and restart Vim/Neovim.`,
+        );
+        console.warn("*".repeat(80));
+      }
+      console.error(`Failed to load file '${path}': ${e}`);
+      throw e;
+    }
+
+    // Import the module (heavy I/O) outside the lock so it can proceed in
+    // parallel with other registerPath calls.
+    const mod: Mod = { mod: undefined, path };
+    try {
+      if (fileInfo.isDirectory) {
+        const entryPoint = join(path, EXT_ENTRY_POINT_FILE);
+        mod.mod = await importPlugin(entryPoint);
+      } else {
+        mod.mod = await importPlugin(path);
+      }
+    } catch (e) {
+      if (isDenoCacheIssueError(e)) {
+        console.warn("*".repeat(80));
+        console.warn(`Deno module cache issue is detected.`);
+        console.warn(
+          `Execute '!deno cache --reload "${path}"' and restart Vim/Neovim.`,
+        );
+        console.warn("*".repeat(80));
+      }
+      console.error(`Failed to load file '${path}': ${e}`);
+      throw e;
+    }
+
+    // Short critical section: update shared state under lock, but re-check to
+    // avoid races (another parallel caller may have registered the same path).
+    await this.#registerLock.lock(async () => {
+      if (path in this.#checkPaths) {
+        return;
+      }
+
+      const typeExt = this.#exts[type];
+      let add;
+      switch (type) {
+        case "ext":
+          add = (name: string) => {
+            const ext = new mod.mod.Ext();
+            ext.name = name;
+            ext.path = path;
+            typeExt[name] = ext;
+          };
+          break;
+        case "protocol":
+          add = (name: string) => {
+            const e...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

Copilot AI changed the title [WIP] Improve I/O performance of linkPath function perf(utils): optimize linkPath with safeStat consolidation and batched parallel directory traversal Apr 5, 2026
Copilot AI requested a review from Shougo April 5, 2026 04:11
@Shougo Shougo marked this pull request as ready for review April 5, 2026 04:50
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