fread: improve performance for unbuffered reads

[pfg]

Summary

We can use the buffer passed to fread(3) directly in the FILE *.
The buffer needs to be reset before each call to __srefill().
This preserves the expected behavior in all cases.

THe change was found originally in OpenBSD and later adopted by NetBSD.

Obtained from: OpenBSD (CVS 1.18)

[kib]

In principle, the optimization is perhaps useful for unbuffered consumers.

The locking in your patch is incompatible with the FreeBSD implementation. Test the patch in multithreaded program.

lib/libc/stdio/fread.c

128

This cannot be right.

[pfg]

In D30548#686024, @kib wrote:

In principle, the optimization is perhaps useful for unbuffered consumers.

The locking in your patch is incompatible with the FreeBSD implementation. Test the patch in multithreaded program.

Bah .. you are right, I am quite rusty on FreeBSD and I just copy-pasted. This shouldn't have compiled.

[kib]

In D30548#686034, @pfg wrote:

In D30548#686024, @kib wrote:

In principle, the optimization is perhaps useful for unbuffered consumers.

The locking in your patch is incompatible with the FreeBSD implementation. Test the patch in multithreaded program.

Bah .. you are right, I am quite rusty on FreeBSD and I just copy-pasted. This shouldn't have compiled.

It should compile fine, but it cannot work.

What you need to do is to remove the wrong unlock line.

[pfg]

In D30548#686179, @kib wrote:

In D30548#686034, @pfg wrote:

In D30548#686024, @kib wrote:

In principle, the optimization is perhaps useful for unbuffered consumers.

The locking in your patch is incompatible with the FreeBSD implementation. Test the patch in multithreaded program.

Bah .. you are right, I am quite rusty on FreeBSD and I just copy-pasted. This shouldn't have compiled.

It should compile fine, but it cannot work.

What you need to do is to remove the wrong unlock line.

Remove or replace?

The change seems to have made an important difference in other BSDs but I'd honestly love to have someone that knows more about locking take over ;).

lib/libc/stdio/fread.c

128

Should it just be?

FLOCKFILE_CANCELSAFE(fp);

[pfg]

lib/libc/stdio/fread.c

128

I meant

FUNLOCKFILE_CANCELSAFE();

but I guess one has to lock before unlocking ?

[kib]

lib/libc/stdio/fread.c

128

Look at fread() and __fread(). You patched __fread(), which assumes that the locks are taken externally and unlocked externally as well, when needed.

In other words, just delete (any) unlock you added.

[pfg]

Duh ! I see now.

Will fix soon. and run some more testing in my system. Thanks!

[pfg]

Remove deprecated unlocking, pointed out by kib

[kib]

This looks fine to me, but I did not tested.

[jhb]

Commit log nit: s/THe/The/