I reverted this in r1580926. Bert Huijben wrote:
>> Log: >> * subversion/svn/notify.c >> (svn_io_sleep_for_timestamps): Simplify, eliminating a return path that >> looked >> like a potential source of bugs but was probably in fact safe. > > As noted on IRC, this patch can make us wait for the next... next second... > by > determining to what second we wait later. It effectively added one 'stat' to the code path for whatever client-level operation was performed. > I don't see why this patch improves the current behavior of this function. It doesn't, it was only to simplify the code and remove what looked like a dodgy return path. > The if case you removed checked if the time to wait for is in the past, which > may happen for various reasons... E.g.g when the stat operation is somehow > slow... or because the OS-scheduler paged us out and only returned later. As I explained on IRC, my version eliminated the possibility of calculating a negative delay. > In these cases we now just wait one second extra... No, not one extra second; we just wait till the next one-second boundary starting from after the 'stat' rather than starting from before the 'stat'. That adds one 'stat' time on average. > Originally we always waited (<= 1.5), but then we made use of the delay time > to determine if we really had to wait. Doing this with this code made sure > the > new code was not *more expensive* than the old check. Agreed. And that was a nice touch. > But now the code is updated to still wait the old long time... after we spend > time determining whether we should wait at all. I've reverted it because I want you to be happy with it and I don't care that much -- the whole thing is a horribly crude hack anyway. If we care about it at all we should make it figure out how long it really needs to wait. p.s. For other readers, I only started looking at this at all because Philip reported an actual bug whereby the function's 1-millisecond delay is simply wrong for many Linux systems. - Julian
