stanio commented on code in PR #466:
URL: 
https://github.com/apache/httpcomponents-client/pull/466#discussion_r1267729731


##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpCache.java:
##########
@@ -313,36 +324,64 @@ public CacheHit storeReusing(
         return store(request, originResponse, requestSent, responseReceived, 
rootKey, hit.entry);
     }
 
-    @Override
-    public void flushCacheEntriesFor(final HttpHost host, final HttpRequest 
request) {
-        final String rootKey = cacheKeyGenerator.generateKey(host, request);
+    private void evictAll(final HttpCacheEntry root, final String rootKey) {
         if (LOG.isDebugEnabled()) {
-            LOG.debug("Flush cache entries: {}", rootKey);
+            LOG.debug("Evicting root cache entry {}", rootKey);
         }
-        try {
-            storage.removeEntry(rootKey);
-        } catch (final ResourceIOException ex) {
-            if (LOG.isWarnEnabled()) {
-                LOG.warn("I/O error removing cache entry with key {}", 
rootKey);
+        removeInternal(rootKey);
+        if (root.isVariantRoot()) {
+            for (final String variantKey : root.getVariantMap().values()) {
+                LOG.debug("Evicting variant cache entry {}", variantKey);

Review Comment:
   > Consider wrapping all debug log statements with a ´if 
(LOG.isDebugEnabled())´ check for performance enhancement by avoiding 
unnecessary string concatenation when debug logging is not enabled.
   
   There's no string concatenation/formatting happening with parametrized 
messages when the target log level is not enabled:
   
   -   [What is the fastest way of (not) 
logging?](https://slf4j.org/faq.html#logging_performance)



##########
httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/BasicHttpCache.java:
##########
@@ -313,36 +324,64 @@ public CacheHit storeReusing(
         return store(request, originResponse, requestSent, responseReceived, 
rootKey, hit.entry);
     }
 
-    @Override
-    public void flushCacheEntriesFor(final HttpHost host, final HttpRequest 
request) {
-        final String rootKey = cacheKeyGenerator.generateKey(host, request);
+    private void evictAll(final HttpCacheEntry root, final String rootKey) {
         if (LOG.isDebugEnabled()) {
-            LOG.debug("Flush cache entries: {}", rootKey);
+            LOG.debug("Evicting root cache entry {}", rootKey);
         }
-        try {
-            storage.removeEntry(rootKey);
-        } catch (final ResourceIOException ex) {
-            if (LOG.isWarnEnabled()) {
-                LOG.warn("I/O error removing cache entry with key {}", 
rootKey);
+        removeInternal(rootKey);
+        if (root.isVariantRoot()) {
+            for (final String variantKey : root.getVariantMap().values()) {
+                LOG.debug("Evicting variant cache entry {}", variantKey);

Review Comment:
   > Consider wrapping all debug log statements with a ´if 
(LOG.isDebugEnabled())´ check for performance enhancement by avoiding 
unnecessary string concatenation when debug logging is not enabled.
   
   There's no string concatenation/formatting happening with parametrized 
messages when the target log level is not enabled:
   
   -   [What is the fastest way of (not) 
logging?](https://slf4j.org/faq.html#logging_performance)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org

Reply via email to