226733 - Upgrade to zlib 1.2.1

Status: VERIFIED FIXED

(SeaMonkey :: General, enhancement)

Description

[Olivier Cahagne]

I couldn't find such a bug report yet, zlib 1.2.1 has noticeable gains in
inflate (20%) and crc32 (50%) functions, according to its author, which could
benefit Mozilla with the XPI, PNG code.

Trunk is currently using 1.1.4 according to:
http://lxr.mozilla.org/seamonkey/source/modules/zlib/src/README

The new DLL could also improve build setup on Win32 (not sure though)
http://www.gzip.org/zlib/DLL_FAQ.txt and perhaps avoid bugs such as bug 167515.

Comment 1

[Giacomo Magnini]

As a side note: nss still uses zlib 1.0.4 (with known security issues, see bug
131826 ), so it would help to do it all at once (if possible).

Comment 2

[tor]

nss uses 1.0.4 with known security fixes backported.

Comment 3

[Glenn Randers-Pehrson]

zlib-1.2.1 seems to be a bit fatter than zlib-1.1.4.  On my FreeBSD platform it
produces a 60,448 byte libmozz.so versus 49,360 bytes produced by zlib-1.1.4.
The tradeoff of size for decompression speed is probably worth while, though.

Comment 4

[Stephen Walker]

Attachment: Upgrade to zlib 1.2.1

Comment 5

[Michael Moy]

I don't know how I got added to this bug but I'm glad to see it. I've been doing
builds with the crc32.c and crc32.h files stolen from zlib 1.2.1 for a little
while and there are no problems. I was going to get around to trying the whole
thing but had to find the glue pieces between Mozilla and Zlib (which I did
eventually find).

Part of the larger lib size is probably due to the additional tables generated
in crc32.h by crc32.c.

BTW, there are three ASM routines for match, gvmat and inffas32 in the masm directories. I don't see any ASM routines in the mozilla\modules\zlib\src
directory so I assume that these processor-family-specific improvements
(potential anyways unless someone has done some testing) aren't in current
Mozilla. Might be interesting to try building these for Intel and/or MMX
as appropriate. We can always steal the extern for MMX from the JPEG code.

Comment 6

[Michael Moy]

One more thing: there's a cut and paste copy of the old csc32 routine renamed
as csc32_convert in memcache.c. The routines in that module seem to be called
by a bunch of ldap routines. I'm going to change it in my local sandbox and
do a build to see what happens.

Comment 7

[Stephen Walker]

Attachment: Upgrade to zlib 1.2.1

Include minigzip.c & README in the diff this time...

Comment 8

[Olivier Cahagne]

is it wise to request reviews already (it doesn't seem to break anything afaik,
given the mmoy builds) ?

Comment 9

[Stephen Walker]

Comment on attachment 143755 [details] [diff] [review]
Upgrade to zlib 1.2.1

No.

It won't happen until 1.8 anyways, and the patch needs a def file so that a
static xpinstall actually links.

Comment 10

[Olivier Cahagne]

mmoy, have you also tested PNG rendering performance (similar to bug 242145
comment 15) ?

Comment 11

[Michael Moy]

No I haven't. I've spent most of my time on JPEG performance. Have looked at the GIF code recently but it doesn't look like there are a lot of easy wins there. There are some though.

I think that PNG rendering in general is very good.

One other problem with PNG performance testing is that it's hard finding large
PNG files. I'll get around to looking at the PNG code at to see if there's anything particularly bad in there.

Comment 12

[tor]

Attachment: Upgrade to zlib 1.2.1

Comment 13

[Glenn Randers-Pehrson]

 In zconf.h, 
 
 #ifdef Z_PREFIX
 #  define deflateInit_  z_deflateInit_
 #  define deflate       z_deflate
 ...
 #  define voidp         z_voidp
 #endif

should be

 #define deflateInit MOZ_Z_deflateinit_
 #define deflate     MOZ_Z_DEFLATE
 ...
 #define voidp       MOZ_Z_voidp

For rationale, see bug #119934 and bug #181936

We haven't seen similar problems with zlib, but that is probably because
prior to release of zlib-1.2.x, the zlib releases have been binary compatible.

Comment 14

[Glenn Randers-Pehrson]

Please excuse the typos in my previous message:
 #define deflateInit MOZ_Z_deflateinit_
 #define deflate     MOZ_Z_DEFLATE

They should read:
 #define deflateInit_ MOZ_Z_deflateInit_
 #define deflate      MOZ_Z_deflate

Comment 15

[tor]

zlib.def for win32 exports a number of symbols not in that prefix table - do
they also need prefixing?

Comment 16

[Glenn Randers-Pehrson]

I would think so.  That may mean zlib is buggy though because the prefix
list should be consistent with the definition list.  I'll ping the zlib
guys about it.

Comment 17

[tor]

Attachment: Upgrade to zlib 1.2.1, make xpinstall happy, no prefixing yet (gz)

Comment 18

[Glenn Randers-Pehrson]

Blocking bug #119934

Comment 19

[Glenn Randers-Pehrson]

Re comment #15 and comment #16, the Z_PREFIX list is incomplete while the
zlib.def list is OK.  We should use the zlib.def list as the basis
for building our mozzconf.h.

Comment 20

[tor]

Attachment: Upgrade to zlib 1.2.1, add prefixing (gz)

Comment 21

[Glenn Randers-Pehrson]

The latest patch (#150035) works for me.

"strings libmozz.so" reveals 12 functions that didn't get the MOZ_Z
prefix.  I believe they are all "private" functions that would never
be referenced by applications, so the lack of prefixing shouldn't matter
much.

_dist_code
_length_code
_tr_align
_tr_flush_block
_tr_init
_tr_stored_block
_tr_tally
deflate_copyright
inflate_copyright
inflate_fast
inflate_table
z_errmsg

Comment 22

[tor]

There's also these two:

00009927 T zcalloc
00009949 T zcfree

Comment 23

[tor]

Attachment: Uprade to zlib 1.2.1, full prefixing (gz)

Comment 24

[tor]

Comment on attachment 150278 [details]
Uprade to zlib 1.2.1, full prefixing (gz)

This increases libmozz by about 11K on linux, but is supposed to
be much faster - 20% inflate and 50% crc32 speedup.  Should benefit
jar, gzip content encoding, png, and xpi.

Comment 25

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 150278 [details]
Uprade to zlib 1.2.1, full prefixing (gz)

r/rs=roc

Comment 26

[tor]

Checked in.  Had to turn off ZLIB_DLL because of linkage problems with the
static build (firefox).

Comment 27

[Hiro]

An error comes out by build of Camino.
zlib of a system is used in build of Camino.
http://www.mozilla.org/ports/fizzilla/ChimChim.html

c++  -fno-rtti -fno-exceptions -Wall -Wconversion -Wpointer-arith -Wcast-align
-Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor
-Wno-long-long -I/Developer/SDKs/MacOSX10.2.8.sdk/usr/include -O2
-fpascal-strings -no-cpp-precomp -fno-common -fshort-wchar
-I/Developer/SDKs/MacOSX10.2.8.sdk/Developer/Headers/FlatCarbon -pipe  -DNDEBUG
-DTRIMMED -O2 -fPIC -arch ppc -o libnecko.dylib  nsNetModule.o  nsIOThreadPool.o
nsTransportUtils.o nsAsyncStreamCopier.o nsAsyncStreamListener.o
nsBufferedStreams.o nsDirectoryIndexStream.o nsDownloader.o nsFileStreams.o
nsInputStreamChannel.o nsInputStreamPump.o nsStreamTransportService.o
nsIOService.o nsLoadGroup.o nsMIMEInputStream.o nsProtocolProxyService.o
nsRequestObserverProxy.o nsResumableEntityID.o nsSimpleStreamListener.o
nsSimpleURI.o nsStandardURL.o nsSocketTransport2.o nsSocketTransportService2.o
nsServerSocket.o nsStreamListenerTee.o nsStreamLoader.o nsSyncStreamListener.o
nsUnicharStreamLoader.o nsURIChecker.o nsURLHelper.o nsURLParsers.o
nsURLHelperOSX.o race.o nameprep.o punycode.o nsIDNService.o nsDNSService2.o
nsHostResolver.o nsSocketProviderService.o nsSOCKSSocketProvider.o
nsSOCKSIOLayer.o nsCookie.o nsCookieService.o nsStreamConverterService.o
nsAppleFileDecoder.o mozTXTToHTMLConv.o nsUnknownDecoder.o nsHTTPCompressConv.o
nsTXTToHTMLConv.o nsDirIndex.o nsDirIndexParser.o nsIndexedToHTML.o
nsMultiMixedConv.o ParseFTPList.o nsFTPDirListingConv.o nsGopherDirListingConv.o
nsMIMEHeaderParamImpl.o nsCache.o nsCacheEntry.o nsCacheEntryDescriptor.o
nsCacheMetaData.o nsCacheService.o nsCacheSession.o nsMemoryCacheDevice.o
nsDiskCacheBinding.o nsDiskCacheBlockFile.o nsDiskCacheDevice.o
nsDiskCacheEntry.o nsDiskCacheMap.o nsDiskCacheStreams.o
nsAboutProtocolHandler.o nsAboutBlank.o nsAboutBloat.o nsAboutCache.o
nsAboutCacheEntry.o nsAboutRedirector.o nsFileProtocolHandler.o nsFileChannel.o
nsFtpProtocolHandler.o nsFTPChannel.o nsFtpConnectionThread.o
nsFtpControlConnection.o nsHttp.o nsHttpHeaderArray.o nsHttpConnectionInfo.o
nsHttpConnection.o nsHttpConnectionMgr.o nsHttpRequestHead.o
nsHttpResponseHead.o nsHttpChunkedDecoder.o nsHttpAuthCache.o
nsHttpAuthManager.o nsHttpBasicAuth.o nsHttpDigestAuth.o nsHttpNTLMAuth.o
nsHttpTransaction.o nsHttpHandler.o nsHttpChannel.o nsHttpPipeline.o
nsJARProtocolHandler.o nsJARChannel.o nsJARURI.o nsResProtocolHandler.o    
-L../../dist/bin -L../../dist/lib  ../../dist/lib/libunicharutil_s.a
-L../../dist/bin -lxpcom -L../../dist/bin
-L/Users/sek/Documents/mozilla-current/camino/mozilla/dist/lib -lplds4 -lplc4
-lnspr4 -lpthread  -L../../dist/lib -lmozz  -framework Carbon 
-Wl,-exported_symbols_list
-Wl,../../build/unix/gnu-ld-scripts/components-export-list -bundle
-L/Developer/SDKs/MacOSX10.2.8.sdk/usr/lib/gcc/darwin/3.3
-L/Developer/SDKs/MacOSX10.2.8.sdk/usr/lib -lm     
ld: warning -prebind has no effect with -bundle
ld: nsHTTPCompressConv.o illegal reference to symbol: _inflateInit_ defined in
indirectly referenced dynamic library
/Developer/SDKs/MacOSX10.2.8.sdk/usr/lib/libz.1.1.3.dylib
make[5]: *** [libnecko.dylib] Error 1
make[5]: Leaving directory
`/Users/sek/Documents/mozilla-current/camino/mozilla/netwerk/build'

Comment 28

[Glenn Randers-Pehrson]

Your system zlib (1.1.3) is too old.  Either update it or use the embedded
zlib.

Comment 29

[tor]

The build dependency system doesn't gracefully handle switching from native
zlib (specified by recommended Caminio build flags) to the tree version (which
happens because this version is now newer).  Do a clobber build of your tree
and it should compile fine.

Comment 30

[Glenn Randers-Pehrson]

Re my comment #28
Even though the system library is old it should be possible to build with it.
I wonder about the "-lmozz" in the build script.  Isn't libmozz the
embedded zlib, not the system zlib?

Comment 31

[Hiro]

>Your system zlib (1.1.3) is too old.  Either update it or use the embedded
zlib.
I am using the system of the newest version.
(Mac OS X 10.3.4)
Therefore, zlib of a system is the newest version.

Although the following options were set to disable, in the build environment of
the present mozilla, priority is given to zlib of a system and it is found.
Therefore, an error is still generated.
  ac_add_options --with-system-zlib

In order to build Camino, is large-scale correction required?

Comment 32

[Glenn Randers-Pehrson]

Re: comment #31
No, all you should need to do is change (in your .mozconfig)
ac_add_options --with-system-zlib
to
ac_add_options --without-system-zlib

That may help you work around the problem.

Comment 33

[Glenn Randers-Pehrson]

I believe Mozilla's configure is now rejecting any system zlib older
than 1.2.1 (as defined in MOZZLIB).  However, I believe 1.1.4 should also be
acceptable.  Your version 1.1.3 is, as I said before, too old.  It has
security vulnerabilities and was replaced by 1.1.4 in March 2002.

Comment 34

[Mike Pinkerton (not reading bugmail)]

osx 10.3.x has 1.1.4. can we get this ok'd on the list that is accepted?

fwiw, the camino static builds are still fubar'd and we haven't had a build for
a week because of this.

Comment 35

[Glenn Randers-Pehrson]

Mike: can you build camino if you change MOZZLIB=1.2.1 to MOZZLIB=1.1.4 in
configure and configure.in?

Comment 36

[Mike Pinkerton (not reading bugmail)]

yes, that works for me after a make clean. tor, can you make that change ASAP so
we get camino nightly builds again? thanks!

Comment 37

[tor]

For consistency across platforms and to help aid tracking/fixing possible
problems with zlib, I'd prefer to keep the zlib configure test the way
it is.

As I said before, a clobber build should be sufficient to get a 
--with-system-zlib tinderbox cycling green again.  If that doesn't work,
let me know.

Comment 38

[Mike Pinkerton (not reading bugmail)]

bryner recollects there were issues with using libmozz with tbird forcing mscott
to use the system zlib on mac. he's checking what those were. We may not have a
choice on OSX.

Would have been nice to know about this before it going in and leaving us
without builds for many days.

Comment 39

[tor]

I didn't know that Camino used --with-system-zlib or that our build system
wouldn't handle the switchover, so I couldn't have given you advance warning.

Comment 40

[Mike Pinkerton (not reading bugmail)]

pinkerton: did you remember what that zlib issue with tbird was?
bryner: i believe it was a multiply defined symbol
bryner: that was causing the link to fail

So this affects more than just camino. We have an issue with static builds here.
cc'ing mscott.

Comment 41

[Scott MacGregor]

pink tbird static builds were breaking on certain machines but not on others.
One of the release machines needed --with-system-zlib in order to make a static
build otherwise I had mozz link errors. Another machine built release static
builds just fine without forcing system zlib. Is that what you were asking about? 

Comment 42

[tor]

The problem with Camino seems to be that the project builder file never
tries to pull in libmozz.  I'm unfamiliar with project builder and if
it's possible to add conditionals to its project description.

Comment 43

[tor]

Incidentally this means that non-system zlib Camino builds are working more by
accident than anything else - the tree builds against the tree zlib headers,
but gets implicitly linked to the system zlib library.

Comment 44

[Mike Pinkerton (not reading bugmail)]

they may build, but they don't work. they crash at startup with no stack.

Comment 45

[Glenn Randers-Pehrson]

If you install zlib-1.2.1 on your system, does Camino successfully build
and run using it?

Comment 46

[Mike Pinkerton (not reading bugmail)]

i have a fix for our dylib builds to run again with libmozz. i still don't know
if our static builds will work. i'll start a build now.

Comment 47

[Mike Pinkerton (not reading bugmail)]

i've landed the changes to fix camino (shared and static builds) to use libmozz.
bryner still says tbird will have problems with static builds, but is entirely
apathetic at this point.

Comment 48

[Glenn Randers-Pehrson]

Re comment #37

But see bug #194665 where it was decided to be lenient in the library
requirements, accepting the earliest version that is compatible.

Comment 49

[Hiro]

(In reply to comment #46)
> i have a fix for our dylib builds to run again with libmozz. i still don't know
> if our static builds will work. i'll start a build now.

libmozz.dylib was not copied to Camino.app.
When libmozz.dylib was copied to Camino.app/Contents/MacOS by the manual, Camino
worked.

Comment 50

[Colin Blake]

Huh? As part of the checkin to zutil.h you removed a "#ifndef VMS" and broke the
VMS build!!!!

Intentional?

Comment 51

[tor]

VMS change checked in.  Please report this bug upstream so we don't have to
go through this again.

Comment 52

[Colin Blake]

> VMS change checked in.  Please report this bug upstream so we don't have to
> go through this again.

Thanks. This was originally addressed by bug 86323 back in 2001.

Comment 53

[Glenn Randers-Pehrson]

I have reported the VMS prototype problem to the zlib developers.