Skip to content

perf(http/file_server): read fileinfo in parallel#3363

Merged
kt3k merged 2 commits intodenoland:mainfrom
ayame113:file-server-readdir-parallel
May 6, 2023
Merged

perf(http/file_server): read fileinfo in parallel#3363
kt3k merged 2 commits intodenoland:mainfrom
ayame113:file-server-readdir-parallel

Conversation

@ayame113
Copy link
Contributor

@ayame113 ayame113 commented May 5, 2023 •
edited
Loading

part of #3361

Previously, fileInfo was read serially when generating directory listing pages. Change this to read in parallel.
After this PR, the display of the directory listing (page below) is tens of milliseconds faster.

image

// When there are 100 files in the directory:
benchmark      time (avg)             (min … max)       p75       p99      p995
------------------------------------------------- -----------------------------
before     29.6 ms/iter (20.56 ms … 34.89 ms) 31.05 ms 34.89 ms 34.89 ms
after      4.27 ms/iter   (3.51 ms … 6.63 ms)  4.44 ms  5.27 ms  6.63 ms

// When there are 1000 files in the directory:
benchmark      time (avg)             (min … max)       p75       p99      p995
------------------------------------------------- -----------------------------
before     310.22 ms/iter (295.08 ms … 331.81 ms) 317.34 ms 331.81 ms 331.81 ms
after       30.35 ms/iter   (28.24 ms … 32.86 ms)  31.65 ms  32.86 ms  32.86 ms
benchmark code: 
// I created 100 files in the testdata/bench directory and run the `deno bench` command.

import { serveDir } from "./file_server.ts";
import { serveDir as serveDirOld } from "./file_server_old.ts";

Deno.bench(async function after() {
  const res = await serveDir(
    new Request("http://localhost:4507/http/testdata/bench/"),
    { quiet: true, showDirListing: true },
  );
  await res.text();
});

Deno.bench(async function before() {
  const res = await serveDirOld(
    new Request("http://localhost:4507/http/testdata/bench/"),
    { quiet: true, showDirListing: true },
  );
  await res.text();
});

@ayame113 ayame113 requested a review from kt3k as a code owner May 5, 2023 17:45
@ayame113 ayame113 mentioned this pull request May 5, 2023
Closed
kt3k
kt3k approved these changes May 6, 2023
View reviewed changes
Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

kt3k
kt3k reviewed May 6, 2023
View reviewed changes
http/file_server.ts
return createCommonResponse(Status.OK, file.readable, { headers });
}

// TODO(bartlomieju): simplify this after deno.stat and deno.readDir are fixed
Copy link
Member

@kt3k kt3k May 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This is an ancient comment by Kevin and this doesn't seem relevant anymore. https://github.com/denoland/deno_std/pull/15/files#diff-106086daad2313990285a9c61085eeb5995d479127db151e613f84ea709450c8R104

kt3k
kt3k reviewed May 6, 2023
View reviewed changes
http/file_server.ts
name: string;
}

const encoder = new TextEncoder();
Copy link
Member

@kt3k kt3k May 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Response object take care of encoding, and this is unnecessary now. Nice clean up.

@kt3k kt3k merged commit 163a902 into denoland:main May 6, 2023
@ayame113 ayame113 deleted the file-server-readdir-parallel branch May 6, 2023 10:11
@ayame113
Copy link
Contributor Author

ayame113 commented May 6, 2023

Thank you for your review! 🙇‍♂️

There is a PR on deno_blog that does a similar optimization. If you don't mind, please take a look when you have time.　denoland/deno_blog#116

mxdvl pushed a commit to mxdvl/deno_std that referenced this pull request May 16, 2023
@ayame113 @mxdvl
perf(http/file_server): read fileinfo in parallel (denoland#3363)
0af8cbe
This was referenced Dec 10, 2023
Closed
Merged
Closed
Closed
Closed
Merged
This was referenced Dec 19, 2023
Open
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kt3k kt3k kt3k approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ayame113 @kt3k