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: remove dir redirection when useless in php_fastcgi #6698

Merged
merged 9 commits into from
Nov 21, 2024

Conversation

dunglas
Copy link
Collaborator

@dunglas dunglas commented Nov 18, 2024

Remove the equivalent of this config if neither {http.request.uri.path}/index.php or {path}/index.php appears in try_files, as it useless:

# Add trailing slash for directory requests
@canonicalPath {
	file {path}/index.php
	not path */
}
redir @canonicalPath {path}/ 308

This prevents one useless filesystem access on the hot path.

Similar to dunglas/frankenphp#1180.

@dunglas
Copy link
Collaborator Author

dunglas commented Nov 18, 2024

The goreleaser check failure is a false positive (because of the name of my branch)

@francislavoie
Copy link
Member

francislavoie commented Nov 18, 2024

/cc @mohammed90 re CI, I think we should set GOPROXY=direct, see golang/go#38861 (comment), or apparently GOPROXY='https://proxy.golang.org|direct' according to golang/go#32955 (comment) if we want to keep caching but I'm not sure we do want caching 😅

@mohammed90
Copy link
Member

/cc @mohammed90 re CI, I think we should set GOPROXY=direct, see golang/go#38861 (comment)

Using GOPROXY=direct means we bypass the Go module cache completely. It's unfortunate, especially that the error is from the build flow within goreleaser/xcaddy flow. I know it breaks the workflow of some contributors, especially you @dunglas, but can't we just avoid using slashes.

I wish Go handled them instead of throwing the burden on us... 😕

@dunglas
Copy link
Collaborator Author

dunglas commented Nov 18, 2024

Yes, I'll avoid slashes. I like this convention but Go tools are totally broken when the branch name contains a slash, I always forget about that.

@francislavoie francislavoie added the optimization 📉 Performance or cost improvements label Nov 18, 2024
@francislavoie francislavoie added this to the v2.9.0-beta.4 milestone Nov 18, 2024
@mholt mholt merged commit eddbccd into caddyserver:master Nov 21, 2024
32 of 33 checks passed
@mholt
Copy link
Member

mholt commented Nov 21, 2024

Thanks for "backporting" the optimization! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
optimization 📉 Performance or cost improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants