ra@is.uu.no (Robert Andersson) (10/26/89)
After I applied patch 18 I watched performance drop and the
output from sar showed that rnews was running read system calls
like mad.
I think the problem is in the checkbatch() function in ifuncs.c.
It depends on setbuf() being functional even after read's have
been performed on the FILE pointer. Well, TFM for my system V.2
based system tells me setbuf() only works directly after an open.
So, the unbatcher ends of doing one read system call pr. byte,
since the setbuf(infp, (char *)NULL)) at the beginning of the
function disables buffering completely.
Bug or feature?
--
Robert Andersson, International Systems, Oslo, Norway
Internet: ra@is.uu.no
UUCP: ...!{uunet,mcvax,ifi}!is.uu.no!rara@is.uu.no (Robert Andersson) (10/29/89)
In article <418@isncr.is.uu.no>, I wrote: >After I applied patch 18 I watched performance drop and the >output from sar showed that rnews was running read system calls >like mad. Since I posted this I have received two replies from: Tom Neff <tneff@bfmny0.uucp> John Limpert <johnl@n3dmc.uucp> Both say they have noticed the same symptoms. John even said he had posted a message similar to mine a while ago, but got no replies. patch 18 added a setbuf() call in ifuncs.c that basically makes all unbatching unbuffered, ie. one read() system call pr. character rather than one pr. BUFSIZ characters. The offending line is line 1454 in ifuncs.c *after* patch 18 has been applied. Who added this line? Why was it done? IMHO *all* setbuf calls in ifuncs.c can be removed without harm. This should make the reading buffered again, and we might not see rnews doing 300 read() system calls / second anymore. -- Robert Andersson, International Systems, Oslo, Norway Internet: ra@is.uu.no UUCP: ...!{uunet,mcvax,ifi}!is.uu.no!ra
rick@uunet.UU.NET (Rick Adams) (10/29/89)
> IMHO *all* setbuf calls in ifuncs.c can be removed without harm. This
Then you obviously have absolutely no idea what you are talking about.
DON'T DO IT. I believe I have fixed the problem. When its run
for a few days here without trashing anything I'll share it.
--rickra@is.uu.no (Robert Andersson) (10/30/89)
rick@uunet.UU.NET (Rick Adams) writes: >> IMHO *all* setbuf calls in ifuncs.c can be removed without harm. >Then you obviously have absolutely no idea what you are talking about. Fine, that might well be true. And after reading your message it's still true. Why so aggressive? I and others have found what we believe is a problem, I am merely trying to shed some light on it. Since you obviously know what you are talking about, why not enlighten us that don't, instead of slapping us in the face? I have taken another look at ifuncs.c now. I see why removing *all* setbuf calls is not a good thing. The input_pipe() function passes file descriptor 0 on to compress. If checkbatch() had used a buffered stdio stream to read the #! cunbatch header it would also effectively had eaten up the first BUFSIZ characters in the file. The compress -d would then fail. Thus buffering is turned off for the initial input. Since other #! headers then rnews could also occur after the initial one this also applies to the output from compress. Thus buffering is turned off there as well. I still have a few questions: 1. Why the setbuf() call that was added by patch 18 in line 1454? It turns off buffering on the final FILE pointer pointing to just the one article which the forked rnews will process. Why? 2. Why use stdio at all? Wouldn't performance improve if checkbatch() instead used read(2)? Then all buffering problems would be gone, and since the #! rnews header contains a byte count it's easy to hand that directly off to read(2). It might not be that robust though, if you don't trust the #! rnews bytecount. 3. Why use a temporary file if the article is longer than 8192 bytes long. Why not open the pipe and fork the child *before* reading the article and stuff it directly into the pipe as you read? >DON'T DO IT. I believe I have fixed the problem. When its run >for a few days here without trashing anything I'll share it. Good, that's what I wanted to hear. And I don't mind being flamed as long as the problem is solved :-) -- Robert Andersson, International Systems A/S, Oslo, Norway. Internet: ra@is.uu.no UUCP: ...!{uunet,mcvax,ifi}!is.uu.no!ra