On Mon, 25 Jun 2007, Linus Torvalds wrote: > > There's also another issue: regular files really *are* different from > pipes and sockets and other things. Not because of EPIPE, but because you > want different buffering behaviour. For a regular file, we really don't > even care about the line buffering, and we'd actually be better off (from > a performance angle) without it. Just for fun, I tried this out. Doing time git log > logfile on the kernel repo with and without the patch I just sent out, I get: Without: real 0m1.361s user 0m1.312s sys 0m0.040s With: real 0m1.687s user 0m1.392s sys 0m0.284s so doing the extra flushing does actually cost us (it's just fundamentally more expensive to do disk IO on non-block-boundaries). It would be much nicer if we only did it for sockets and pipes, which don't have the same block-boundary issues anyway (there's still the system call cost, but on a pipe/socket, the real costs tend to be elsewhere). Again, this is something that a non-stdio-based buffering library would easily handle. You could just test the file descriptor _once_ at the beginning, to see if it's a regular file or not. And then you could have the error handling where it belongs (when the IO is actually done, and the error actually happens) rather than in the callers using a bad interface that sometimes loses 'errno'. Btw, to balance the above performance comment: doing the flush_or_die() obviously *does* mean that you get better performance in the odd cases. For example, if you do [torvalds@woody linux]$ trap '' SIGPIPE [torvalds@woody linux]$ time git log | head I get get 0.002s, while it used to be: real 0m1.382s user 0m1.340s sys 0m0.028s just because it did the whole thing regardless of any EPIPE errors. Of course, that case probably isn't very usual, but I could imagine that some users of "git blame -C --incremental" could actually cause situations like this (ie just stop listening when they got the part they're interested in, and maybe they'd have some strange reason to ignore SIGPIPE). So I'm not opposed to the patch I sent out, I just wanted to point out that this is an area we *could* improve upon if we didn't do that stdio thing. Linus - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
