bring speed back into KConfig

[Oswald Buddenhagen]

On Fri, Apr 18, 2008 at 12:24:58AM +0200, Jakub Stachowski wrote:
> I did some more optimizing to  minimize copying data around. Instead of using 
> QByteArray everywhere I added class BufferFragment with very similar (bare 
> minimum used by parser) API, but operating on allocated earlier big buffer. 
> like left(), trim(), mid(), etc. are only pointer and int operations.
> 
kinda cool.

i have no real objections to the patch except that you should finally
get used to placing spaces around operators. that legibility thing,
kinda. ;)

On Fri, Apr 18, 2008 at 12:36:51AM +0200, Alexander Neundorf wrote:
> > BufferFragment class contains very short functions (most of them 1-3 lines)
> > that could be inlined, so all definitions are in header file. Is it OK or
> > separate .cpp file is necessary?
> 
> Not sure. Having it inline makes it faster but there can be (binary ?) 
> compatibility issues. I don't know if that matters here, since it's just an 
> internal header.
> 
this doesn't make any sense. ;)
leave it in the .h.

On Fri, Apr 18, 2008 at 01:48:24AM +0200, Dirk Mueller wrote:
> a) the readall() might kill the app if its a really big config file. can you 
> add some "chunking" so that it perhaps reads only 128kb or something like 
> that at a time, even if its a really big kconfig file?
> 
i thought about that as well. but if the readAll is going to kill the
app, then the parsed config likely will, too. given that chunking will
make the code much more complicated and *will* introduce a performance
hit, i'd say leave it as is.

> b) the methods do need clear documentation about the expected state and 
> calling convention, to make the patch easier to review. 
> 
+1
fwiw, instead of
  BufferFragment &foo
for in+out parameters, you might prefer
  BufferFragment *foo
- that makes it more obvious. qt uses that convention, too.

> c) please don't add an implicit QByteArray() operator.. add a toByteArray() 
> function and place the conversion explicitely in the code, so that it is 
> clear when the "slow" conversion is happening. 
> 
+1

-- 
Hi! I'm a .signature virus! Copy me into your ~/.signature, please!
--
Confusion, chaos, panic - my work here is done.