[squid-dev] [PATCH] Accumulate less

Alex Rousskov rousskov at measurement-factory.com
Sun May 1 21:41:53 UTC 2016


On 04/27/2016 06:39 PM, Amos Jeffries wrote:
> On 28/04/2016 5:14 a.m., Alex Rousskov wrote:
>> Hello,
>>
>>     The attached patch changes Squid to accumulate fewer unknown-size
>> responses to avoid overwhelming disks.


> +1. I just have some polish to consider doing on merge.
> 
> in src/MemStore.cc:
> * extraneous empty line removal.
>  - IMO it reads better with that line as-was.

Not done. The empty lines were removed to group the three variables with
the only if-statement that uses them:

>      const int64_t expectedSize = e.mem_obj->expectedReplySize(); // may be < 0
>      const int64_t loadedSize = e.mem_obj->endOffset();
>      const int64_t ramSize = max(loadedSize, expectedSize);
>      if (ramSize > maxObjectSize()) {
>          debugs(20, 5, HERE << "Too big max(" <<
>                 loadedSize << ", " << expectedSize << "): " << e);
>          return false; // will not cache due to cachable entry size limits
>      }

Prior to this patch, there were two if-statements using those variables
so the best line grouping was different. Now, there is just one block.


> in src/fs/ufs/UFSStoreState.cc:
> * please add whitespace around the '>' to improve debugs readability a
> bit. s/">"/" > "/

Done.


> in src/store/Controller.cc:
> * please consider updating to C++ casts or removing entirely the C-style
> casts in updateLimits().

Left as is because there are so many [potential] problems with that old
code that quickly touching it up just to update the wrong casts style
may do more harm than good. Let those ugly casts serve as a red flag to
the reader -- this is old code that needs some love; do not trust it.


Committed to trunk as r14656.


Thank you,

Alex.



More information about the squid-dev mailing list