From: Marco Costalba Date: Sun, 10 Dec 2006 22:35:33 GMT Subject: Re: [RFC \ WISH] Add -o option to git-rev-list Message-ID: In-Reply-To: On 12/10/06, Linus Torvalds wrote: > > > On Sun, 10 Dec 2006, Marco Costalba wrote: > > > > Ok. Perhaps I'm doing something wrong but the following code it's > > always 10% slower then the temporary file one (4.7s against 4.3s for > > linux tree) > > Why do you seem to be doing a "new" on every iteration inside the loop? > Becuase it's there where I store the file content. Function parseSingleBuffer(ba) does only the indexing. But the file content is stored in QByteArray objects (little wrappers around a const char* []). So the fread() in the byte array object is the _only_ data copy operation in whole qgit. > Also, why do you have that strange FILE_BLOCK_SIZE thing, and in > particular the "if (len < FILE_BLOCK_SIZE)" check? One thing that pipes vs > files do is the blocking factor. > > Especially with older kernels, I _guarantee_ you that you'll only ever get > 4kB at a time, so because of that "if (len < 64kB) break" thing, the only > thing you're doing is to make sure things suck performance-wise, and you > won't be reading the rest of the data until 100ms later. > I consistently have len == 65536 bytes until the last fread() where it's less. See below run against qgit own repository with 'len' printed inside while loop. $ ./qgit Found GNU source-highlight 2.5 len is <65536> len is <65536> len is <65536> len is <65536> len is <65536> len is <65536> len is <65536> len is <54479> bash-3.1$ > IOW, your code is written for a file, and makes no sense there either > (checking "feof(file)" is wrong, since you may well have hit the EOF > *at*that*time*, but the file may GROW since you are doing it all in the > background, so you can't rely on feof() anyway). > Yes. feof() it's difficult to handle correctly. > For a pipe, what you should do is to make sure it's in nonblocking mode, > and just continue reading until it gets no more. And instead of using a > timeout, you should use poll() or something to get notified when there is > more data. > How can open in nonblocking mode with popen() ? FILE *popen(const char *command, const char *type); > IOW, the reason it's slow is because you're doing the wrong thing. > That's for sure :-) My problem is to guess what's is wrong.