Re: [PATCH] csum-file: flush less often
flat view
From: Junio C Hamano <hidden>
Date: 2021-03-25 18:53:29
Junio C Hamano [off-list ref] writes:
quoted
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.