Skip to content

gh-151788: Speed up http.server directory listing by using os.scandir() #151789

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mjbommar wants to merge 5 commits into
base: main
Choose a base branch
from
+23 −10
Open

gh-151788: Speed up http.server directory listing by using os.scandir() #151789

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0606d02
gh-151788: Speed up http.server directory listing by using os.scandir
mjbommar Jun 20, 2026
9ba9f68
gh-151788: Address review — OSError fallback and tone down NEWS
mjbommar Jun 20, 2026
57bc982
gh-151788: Address review — drop perf comment, tame NEWS wording
mjbommar Jun 20, 2026
1138200
gh-151788: Fix NEWS entry doc reference
mjbommar Jun 21, 2026
d47f9fb
gh-151788: Apply review suggestions
mjbommar Jun 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions Lib/http/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -872,13 +872,13 @@ def list_directory(self, path):

"""
try:
list = os.listdir(path)
with os.scandir(path) as it:
entries = sorted(it, key=lambda e: e.name.lower())
except OSError:
self.send_error(
HTTPStatus.NOT_FOUND,
"No permission to list directory")
return None
list.sort(key=lambda a: a.lower())
r = []
displaypath = self.path
displaypath = displaypath.split('#', 1)[0]
Expand All @@ -899,15 +899,24 @@ def list_directory(self, path):
r.append(f'<title>{title}</title>\n</head>')
r.append(f'<body>\n<h1>{title}</h1>')
r.append('<hr>\n<ul>')
for name in list:
fullname = os.path.join(path, name)
displayname = linkname = name
for entry in entries:
displayname = linkname = entry.name

@picnixz picnixz Jun 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
displayname = linkname = entry.name
displayname = linkname = name = entry.name

# Ignore any OSError raised by the os.DirEntry methods
# to match the behavior of their os.path.* counterpart.
try:
is_dir = entry.is_dir()
except OSError:
is_dir = False
try:
is_symlink = entry.is_symlink()
except OSError:
is_symlink = False
# Append / for directories or @ for symbolic links
if os.path.isdir(fullname):
displayname = name + "/"
linkname = name + "/"
if os.path.islink(fullname):
displayname = name + "@"
if is_dir:
Comment thread
picnixz marked this conversation as resolved.
displayname = entry.name + "/"
linkname = entry.name + "/"
if is_symlink:
displayname = entry.name + "@"
Comment on lines +916 to +919

@picnixz picnixz Jun 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you avoid having an LLM doing those suggestions please?

@mjbommar mjbommar Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure, AFK today but can do that tonight. Emacs doesn't work so well on phone. You want PR clean without Claude?

@picnixz picnixz Jun 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What I (personally) want usually is:

  • human interaction (I don't want to communicate with an agent, that's out the question. I don't care whether the text has been written by the AI as long as it's not a C/C without any human review before sending it but I don't want an automated conversation)
  • commits that are always reviewed by a human before even committing them; in general this means that the AI is giving you the rough idea or contents but you are responsible for cleaning it up, and addressing things that could help the next review. In this case, I considered the name = entry.name to be un-necessary so reducing the diff only required to keep the same code and inlining that access. It didn't mean to rewrite all the occurrences with entry.name.

Our policy is that AI is a tool that can be used but it's not something meant to generate PRs for users that would later take credit for (I'm not saying you are, I'm saying that just how AI PRs usually go: most of the time, people just want to have a contribution and so they simply leave their agent in automode).

@picnixz picnixz Jun 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So, the problem generally is not the AI, it's more that whenever there is an LLM, that LLM tends to choose things that are either useless, weird, and that would have been avoided if a human had directly written the lines. If you prefer writing your code through Claude, it's ok, but keep in mind that reviewers are not responsible for guiding the AI, it's the contributor. If we keep telling their agent through our review "no, not like that", it's as if we were directly using an agent in the least efficient way, and hence we are both losing our time.

@mjbommar mjbommar Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah. FWIW, Claude pointed out that your suggestion resulted in a NameError at string concat below, so it proposed this as easiest way to avoid name global and I accepted it

@picnixz picnixz Jun 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh, my bad actually it should have been displayname = linkname = name = entry.name. But yeah, in this case, I would have preferred to know why it decided to do so. And then I would have suggested:

displayname = linkname = name = entry.name

instead of the two lines. That's why I prefer that you tell me this first before changing otherwise I would get confused.

@picnixz picnixz Jun 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So to reiterate: there is nothing wrong with using Claude (the only think I don't like about it is the long summaries in every PR and trivial things it reports like "all tests pass" which is something I would expect for a PR otherwise it doesn't make sense). It's just that having a 3rd actor usually creates more back and forth between the reviewer and the reviewee.

Comment on lines +916 to +919

@picnixz picnixz Jun 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
displayname = entry.name + "/"
linkname = entry.name + "/"
if is_symlink:
displayname = entry.name + "@"
displayname = name + "/"
linkname = name + "/"
if is_symlink:
displayname = name + "@"

# Note: a link to a directory displays with @ and links with /
r.append('<li><a href="%s">%s</a></li>'
% (urllib.parse.quote(linkname,
Expand Down
4 changes: 4 additions & 0 deletions Misc/NEWS.d/next/Library/2026-06-20-09-00-26.gh-issue-151788.Gr8IxK.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
:class:`http.server.SimpleHTTPRequestHandler` now relies on :func:`os.scandir`
instead of :func:`os.listdir` to build directory listings, thereby reducing
the total number of :func:`os.stat` calls per entry. This typically improves
directory listing performance on filesystems where such calls are slow.
Loading