Re: [RFC \ WISH] Add -o option to git-rev-list

["Marco Costalba" <mcostalba@gmail.com>]

On 12/10/06, Linus Torvalds <torvalds@osdl.org> 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.