-
Notifications
You must be signed in to change notification settings - Fork 974
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
Performance and code optimization in HTTP cache #496
Conversation
@arturobernalg Please take a look. |
…; avoid parsing DATE header of cache entries and HTTP messages multiple times
…heKeyGenerator; added test cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ok2c The change-set looks good. Just a small comment.
@@ -258,7 +253,7 @@ protected boolean isExplicitlyNonCacheable(final ResponseCacheControl cacheContr | |||
} | |||
|
|||
protected boolean isExplicitlyCacheable(final ResponseCacheControl cacheControl, final HttpResponse response) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're calling isExplicitlyCacheable
twice within the isResponseCacheable
method. Maybe we could storing its result in a variable and reusing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're calling
isExplicitlyCacheable
twice within theisResponseCacheable
method. Maybe we could storing its result in a variable and reusing it.
@arturobernalg True. However, It is relatively cheap and the first invocation gets executed conditionally. I think It is not worth the ugliness of having an extra parameter to pass around.
Caching code optimization
CacheSupport
ResponseCachingPolicy#isResponseCacheable
by parsing responseDATE
andEXPIRES
headers only onceHttpCacheEntry
to cache parsedDATE
,EXPIRES
andLAST_MODIFIED
valuesHttpCacheEntry
to avoid parsingDATE
header of cache entries and HTTP messages multiple times