[Zlib-devel] Mark Adler, where are you?

[Jan Seiffert]

Daniel Richard G. schrieb:
> On Fri, 20 Jul 2012, Jan Seiffert wrote:
> 
>> But you could test if it regresses on other architectures (maybe
>> important to you) then s/390 ;)
> 
> Not much I can do there besides running "make check"...
> 

Maybe it's time for an official "make perf_test"

[snip]
> 
> That's a lot of trouble to go to to make uFastInt a 32-bit integer on
> x86-64. Why not just use uint32_t unconditionally?

Because i wasn't sure. zlib is designed and written to cater to old/small
systems were int is just 16 bit. So i could prop. have choosen
uint16_fastest_t.
A 8bit µC would really hate it to use 32Bit for these counter/index.

> Is there any platform that will run faster with uFastInt being a 64-bit type?
> 

This patch was more to bang some C language foo into place.
The question is not if a CPU is faster doing 64 Bit arith. At best it is
exactly the same (minus really wacky systems).
The Patch is about minimizing type conversions and
sign-/zeroextends mandated by C by using the native register size. For some
CPUs that's an uLong, but not all...
Then the Patch ran into the Problem that a 64Bit CPU can be slower
doing 64 Bit arith.

The key lies in creating a special type in the first place so
you can easily redirect all uses in the program to the right type. 

Choosing a different type for __x86_64__ by ifdefs is a clutch, no doubt,
but at least as long as you can confine the ugliness to one place...

>> Your valgrind cleanup - Here the "problem" is you are doing an
>> (often) 128kb memset (if memory serves me right). And that probably
>> for nothing.
> 
> That memset() follows a malloc(). If inner-loop performance is a
> concern, then the latter is a *much* bigger issue than the former.
> 

Sure, malloc isn't cheap, but note: this is the memory allocator you
pass in with your struct deflate. I for example use a thread local
special allocator, 200 instructions and the alloc is done.
memset a 128kb area needs ca. 128kb / 8 * 3 = 49k instructions on a
64 Bit CPU (if you have a postincrement pointer). That get's worse
very fast.
And general purpose allocators are also often very fast, if
they are worth their salt.

> And it's certainly not for nothing---not for all the times that I've
> been Valgrinding a large application, and the end result is no memory
> leaked, and several thousand uninitialized-bytes errors in one
> context deep within zlib. There's no good reason to keep things like
> that.
> 

How about:
#ifdef DEBUG
# ifdef HAVE_VALGRIND_MEMCHECK_H
	VALGRIND_MAKE_MEM_DEFINED(s->prev, s->w_size * sizeof(Pos));
# else
	zmemzero(s->prev, s->w_size * sizeof(Pos));  /* shuts up checker */
# endif
#endif

Install the debug version of zlib and be happy

>> If you look at deflate.c at 191 (around #define CLEAR_HASH) it says
>> that prev[] will be initialised on the fly. This could be wrong,
>> but i doubt it. Valgrind can sometimes be very tricky. Or it may be
>> an idiom Valgrind can't understand like "Yes, the code does read
>> uninitialised data, but we do nothing with it". (cmp.
>> deflate.c at 1358, btw, did you compile with FASTEST or not?)
> 
> Nope, didn't use FASTEST (described as "deflate algorithm with only
> one, lowest compression level" in zlib.h).
> 
> There is indeed a way that Valgrind can be told "yes, we're reading
> uninitialized data here, but that's okay." And I'm sure that there is
> a way to tell Purify the same thing. It's even possible that there
> exist configuration options to do this for most of the entries in
> 
> http://en.wikipedia.org/wiki/Memory_debugger#List_of_memory_debugging_tools
>
>  Calling memset() is easier :-)
> 

And slow if done unconditionally.
I can already see the patch in two years from Google or some embedded dev
to remove it.

>> It would have been good to include the error output from valgrind.
> 
> You would really have found that helpful?
> 

If it has line numbers, yes.
A small reproducer would also be OK.

> 
> --Daniel
> 
> 

Greetings
	Jan

-- 
A UDP packet walks into a