Re: [PATCH] csum-file: flush less often

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

>> So, I'm of two minds here:
>>
>>  1. This is embarassing. I wasted everyone's time for nothing. I can retract
>>     this patch.
>>
>>  2. This is embarassing. I overstated the problem here. But we might be able
>>     to eke out a tiny performance boost here.
>>
>> I'm open to either. I think we should default to dropping this patch unless
>> someone thinks the rewrite above is a better organization of the logic. (I
>> can then send a v2 including that version and an updated commit message.)
>
> 3. The current code around "if (nr == sizeof(f->buffer))" might be a
>    bit too clever for readers who try to understand what is going
>    on, and the whole "while" loop may deserve a comment based on
>    what you wrote before your replacement implementation.

Having said all that, comparing the original and the version updated
with your "flush less often" patch, I find the latter quite easier
to read, so as long as the update does not give us 1% slowdown, it
may be worth adopting for the readability improvement alone.

Of course, if we were to go that route, the sales pitch in the log
message needs to be updated.

Thanks.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux