Re: fprintf_ln() is slow

[Jeff King <peff@peff.net>]

On Thu, Jun 27, 2019 at 02:00:54PM +0200, Johannes Schindelin wrote:

> > I can get similar speedups by formatting into a buffer:
> [..]
> > But we shouldn't have to resort to that.
> 
> Why not?

My thinking was that it was introducing an extra copy of the data. But
on further reflection, it probably doesn't, at least for the case of an
unbuffered stream (we might want to predicate it on "fp == stderr",
though).

> It would make for a perfectly fine excuse to finally get work going in
> direction of a initially heap-backed strbuf. Which we have wanted for ages
> now.

I assume you initially stack-backed strbuf?

Perhaps. I'm not sure that it's better for the fallback when we need
more than the stack buffer to be allocating more memory as opposed to
just calling fprintf() and sending it out in multiple writes.

> > We can use setvbuf() to toggle buffering back and forth, but I'm not
> > sure if there's a way to query the current buffering scheme for a stdio
> > stream.
> 
> It also is not very safe, especially when we want to have this work in a
> multi-threaded fashion.

I considered that, too, but I think it is safe. stdio has its own
locking, so every individual call is atomic. The potentially problematic
case would be where we switch back from line buffering to no-buffering,
and somebody else has written some content into our stack-based buffer
(that is about to go out of scope!). But I'd assume that as part of the
switch to no-buffering that any stdio implementation would flush out the
buffer that it's detaching from (while under lock). Nothing else makes
sense.

That said...

> I'd be much more comfortable with rendering the string into a buffer and
> then sending that buffer wholesale to stderr.

It's sufficiently complex that I think I prefer to just use our own
buffer, too.

It also makes it more likely for the newline and the message to end up
in an atomic write(), so if multiple threads _are_ writing to stderr
they'd be more likely to stay together.

It does sound like people in the other part of the thread are OK with
just getting rid of the "_ln" functions altogether.

-Peff