From: mja@trudge.engr.sgi.com (Mike Abbott)
Message-Id: <199907122321.QAA41140@trudge.engr.sgi.com>
Subject: Re: [PATCH] 10x performance increase patch #1
To: new-httpd@apache.org
Date: Mon, 12 Jul 1999 16:21:04 -0700 (PDT)
In-Reply-To: <Pine.LNX.3.96dg4.990708172538.1849T-100000@twinlark.arctic.org>
 from "Dean Gaudet" at Jul 8, 99 05:39:09 pm
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 8bit
Sender: new-httpd-owner@apache.org
Precedence: bulk
Reply-To: new-httpd@apache.org
Status: RO

> > 	- my 64-bit port with documentation.  See the new file
> > 	  htdocs/manual/64-bit.html for details.
> 
> Oh no!  :)  We've gone through this before, about 2 years ago.

Oops.  I naively assumed that I was the first to port Apache to 64 bits
because (a) the code was so 64-bit unclean (at least to SGI's
compilers), (b) this comment in src/Configure made it sound like
something new:
	# Note: We'd like to see patches to compile 64-bit, but for now...
(c) I found no mention of 64-bit issues in any of its documentation, and
(d) my involvement with Apache goes back only six months.  I'm reviewing
the mail archive now.

> -    if (server->sin_addr.s_addr != htonl(INADDR_ANY))
> +    if (server->sin_addr.s_addr != (ap_uint32) htonl(INADDR_ANY))
> 
> According to Single Unix, htonl is defined to return uint32_t.  No cast
> should be necessary...

Okay, omit that cast.  Apache compiles cleanly for 64 bits without it.
I added it to avoid accidental int-to-long extension ala INADDR_NONE.

> -               n = fread(buf, sizeof(char), IOBUFSIZE, f);
> +               n = (int) fread(buf, sizeof(char), IOBUFSIZE, f);
> 
> would seem to be better done by changing the type of n to be size_t.

That's another way, sure.

> -   long bytes_written;
> +   ap_int32 bytes_written;
> 
> In general you seem to have chosen the 32-bit quantity when ssize_t
> or size_t would seem far more appropriate...

I chose not to port file sizes, bytes written, content lengths, and the
like to 64 bits for several reasons, including:
	- doing so would have required much more extensive changes
	  (buff.c etc.).
	- doing so would have broken modules' binary compatibility
	  because data structures would have changed size.  This is moot
	  on Irix where 32-bit and 64-bit objects and libraries are
	  mutually incompatible but perhaps it is pertinent on other
	  platforms with which I am unfamiliar.
	- the most popular browsers can't handle 64-bit header values or
	  amounts of content that require 64 bits to measure.
I chose not to port them; someone may port them in the future.

> > 	- a lot of general cleanup to make Apache compile without any
> > 	  warnings from SGI's lint-like compilers.
> 
> Could we just turn off the "argument not used" warnings?

Of course.  I prefer not to though because it silences even the valid
"argument not used" warnings.  Having unused arguments is harmless but I
believe addressing warnings individually results in cleaner code.

> Still perusing... I'm thinking of adopting most of this into mpm.

Cool.

It seems most of the differences you cited can be understood best as
simply programmer's preference.  The goals of this patch are to make
Apache compile warning-free and run on 64-bit Irix.  These goals are
important to me.  I'm certainly willing to bend selecting among
equivalent implementations.
--
Michael J. Abbott    mja@sgi.com