Skip to content
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.

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

perf: improve php_server directive #1180

Merged
merged 5 commits into from
Nov 21, 2024
Merged

perf: improve php_server directive #1180

merged 5 commits into from
Nov 21, 2024

Conversation

dunglas
Copy link
Owner

@dunglas dunglas commented Nov 18, 2024

Prevents some I/O accesses when disabling the file server and when disabling index files in subdirectory support.

caddy/caddy.go Outdated

// if tryFiles wasn't overridden, use a reasonable default
if len(tryFiles) == 0 {
tryFiles = []string{"{http.request.uri.path}", "{http.request.uri.path}/" + indexFile, indexFile}
if disableFsrv {
tryFiles = []string{"{http.request.uri.path}/" + indexFile, indexFile}
Copy link
Owner Author

Choose a reason for hiding this comment

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

This change beaks the tests.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Actually, the new behavior is better and more correct than the previous one. I updated the test accordingly.

@AlliBalliBaba
Copy link
Collaborator

AlliBalliBaba commented Nov 20, 2024

I wonder if it would also make sense to cache the location of existing index files (unless that's something you're already doing here). The only disadvantage would be that when the index file is deleted, we'd get a PHP 500 instead of a 404.

@dunglas
Copy link
Owner Author

dunglas commented Nov 20, 2024

I think it's a good idea. We'll have to introduce a LRU cache in Caddy to do that. It may also be needed for another optimization I have in mind: caching if files (assets) exist or not for some time (configurable).

That being said, I'm currently optimizing for the framework case (Laravel/Symfony), and I think that for this specific (but very common) case I'll be able to get 0 to disk access with caddyserver/caddy#6699 and some extra tricks to come.

@dunglas dunglas merged commit 2d6a299 into main Nov 21, 2024
46 checks passed
@dunglas dunglas deleted the perf/php_server branch November 21, 2024 12:22
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