HTTP::StaticFileHandler: Reduce max stat calls from 6 to 2 #12310
Conversation
| @@ -71,7 +72,7 @@ class HTTP::StaticFileHandler | |||
| context.response.content_type = "text/html" | |||
| directory_listing(context.response, request_path, file_path) | |||
| elsif is_file | |||
| last_modified = modification_time(file_path) | |||
| last_modified = file_info.not_nil!.modification_time | |||
There was a problem hiding this comment.
I think the is_file check can be inlined here, so there's no need for not_nil!
There was a problem hiding this comment.
Oh yeah, that's a good alternative to early return.
There was a problem hiding this comment.
Compiler nil check error without an early return
| context.response.headers["Content-Encoding"] = "gzip" | ||
| end | ||
| end | ||
|
|
||
| context.response.content_length = File.size(file_path) | ||
| context.response.content_length = file_info.not_nil!.size |
There was a problem hiding this comment.
What happens if file_info is nil? Can that happen at all?
There was a problem hiding this comment.
No it can't because the branch is guarded by is_file which require file_info present. not_nil! can still be avoided here, though if we just return early when file_info is nil.
Remove not_nil!
| is_dir = file_info && file_info.directory? | ||
| is_file = file_info && file_info.file? |
There was a problem hiding this comment.
Minor suggestion:
| is_dir = file_info && file_info.directory? | |
| is_file = file_info && file_info.file? | |
| is_dir = file_info.try(&.directory?) | |
| is_file = file_info.try(&.file?) |
There was a problem hiding this comment.
&& is an attempt to reduce the type to Bool.
@asterite Does optimize anything?
There was a problem hiding this comment.
The result of both expression is identical. It's not bool because Nil#&& returns Nil.
See https://crystal-lang.org/reference/1.5/syntax_and_semantics/and.html
There was a problem hiding this comment.
My opinion on these suggestions is that they are just suggestions. It depends on your code style, and the guide says nothing about this. So feel free to pick them up or ignore them.
No description provided.