You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Brian Pane <bp...@pacbell.net> on 2001/09/07 23:23:55 UTC
[PATCH] Turn apr_table_t into a hash table
The attached patches change the apr_table_t implementation from
a linear list to a hash table (not an apr_hash_t, though!). With
this change, I'm seeing a ~3% improvement in throughput when
delivering a 0-byte file over the loopback on Linux. (I used this
0-byte test case to measure the inherent overhead in the httpd, without
transmission time clouding the results.)
Design notes:
* The apr_table_t API is almost completely unchanged. The one
exception is that I removed the apr_array_elts macro and
replaced it with a const iterator API. The second of the
attached patches changes the six places in the httpd-2.0 source
that were impacted by this.
Note: We could probably get even better performance through
more radical reimplementations of the data structure for
request_rec->headers_in, but I chose this more conservative
approach because IMHO it's too late in the 2.0 development
cycle to break that much code. However, this patch does
open the door to more optimizations in the future, by making
apr_table_t an abstract data type.
* The implementation is a chained hash table, with the elements
in each chain kept in sorted order as a small additional
optimization.
* Each element in the hash table uses an array to support the
storage of multiple values for the same key. This complicates
the code a bit, but I wanted to make sure that apr_table_addn()
could still append values in O(1) time instead of O(n^2).
--Brian
Re: [PATCH] Turn apr_table_t into a hash table
Posted by Ryan Bloom <rb...@covalent.net>.
On Saturday 08 September 2001 16:42, Ian Holsman wrote:
> William A. Rowe, Jr. wrote:
> > From: "Brian Pane" <bp...@pacbell.net>
> > Sent: Friday, September 07, 2001 2:23 PM
> >
> >>The attached patches change the apr_table_t implementation from
> >>a linear list to a hash table (not an apr_hash_t, though!). With
> >>this change, I'm seeing a ~3% improvement in throughput when
> >>delivering a 0-byte file over the loopback on Linux. (I used this
> >>0-byte test case to measure the inherent overhead in the httpd, without
> >>transmission time clouding the results.)
> >
> > This breaks ordering, correct? If so, -1. The apr_table_t must remain
> > ordered as pushed onto the array.
> >
> > Bill
>
> Hey Bill.
> Are you concerned about ordering of values inside of a element, or ordering
> of the elements themselves?
It doesn't matter. :-) As everybody tells me whenever I bring up this feature
of tables, it wasn't a design consideration when they were created. The fact
that tables keep elements in order has always been a by-product of the
API, not a design goal.
Now, I personally believe that since tables have always had this element ordering,
we should preserve it moving forward. However, enough others have said that
this doesn't need to be preserved, so I believe we can get past this.
However, I continue to believe that having two different hash API's in a single
library is a mistake. If we want to replace tables with hashes, then replace the
tables with hashes. That makes much more sense when reading the code.
If you want the type safety of knowing that you are putting a string into the
hash, then wrap the current hash API for string specific use. This can be
done with less code changes in APR. It also has the benefit of being able to
read the httpd code, and knowing that you are using a hash table.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: [PATCH] Turn apr_table_t into a hash table
Posted by Ian Holsman <ia...@cnet.com>.
William A. Rowe, Jr. wrote:
> From: "Brian Pane" <bp...@pacbell.net>
> Sent: Friday, September 07, 2001 2:23 PM
>
>
>
>>The attached patches change the apr_table_t implementation from
>>a linear list to a hash table (not an apr_hash_t, though!). With
>>this change, I'm seeing a ~3% improvement in throughput when
>>delivering a 0-byte file over the loopback on Linux. (I used this
>>0-byte test case to measure the inherent overhead in the httpd, without
>>transmission time clouding the results.)
>>
>
> This breaks ordering, correct? If so, -1. The apr_table_t must remain
> ordered as pushed onto the array.
>
> Bill
>
Hey Bill.
Are you concerned about ordering of values inside of a element, or ordering
of the elements themselves?
..Ian
Re: [PATCH] Turn apr_table_t into a hash table
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
From: "Brian Pane" <bp...@pacbell.net>
Sent: Friday, September 07, 2001 2:23 PM
> The attached patches change the apr_table_t implementation from
> a linear list to a hash table (not an apr_hash_t, though!). With
> this change, I'm seeing a ~3% improvement in throughput when
> delivering a 0-byte file over the loopback on Linux. (I used this
> 0-byte test case to measure the inherent overhead in the httpd, without
> transmission time clouding the results.)
This breaks ordering, correct? If so, -1. The apr_table_t must remain
ordered as pushed onto the array.
Bill
Re: [PATCH] Turn apr_table_t into a hash table
Posted by Brian Pane <bp...@pacbell.net>.
Ryan Bloom wrote:
>On Saturday 08 September 2001 15:25, Brian Pane wrote:
>
>>Ryan Bloom wrote:
>>
>>>>>The latter. Having two API's to the same functions should only be done
>>>>>as a stop-gap.
>>>>>
>>>>I disagree. It's inevitable to have two APIs, as long as we have two
>>>>'table' types with very different semantics.
>>>>
>>>>apr_table_t is statically typed (uses char*), and apr_hash_t isn't (uses
>>>>void*).
>>>>If we did a direct replacement of apr_table_t with apr_hash_t in the
>>>>httpd, we'd be sacrificing static type-safety for a lot of code.
>>>>
>>>So create a simple set of prototypes and macros for a char * version of
>>>the hash code, the same way we did for ap_strchr and ap_strchr_c. If you
>>>do that, you will be adding probably two or three new functions to APR.\
>>>
>>Yes, macro wrappers would work. But if we use that approach, it would
>>be advantageous to use macro names like "apr_table_get" so that all
>>the existing httpd code and 3rd party modules can be used without
>>modification.
>>This would be functionally equivalent to the "implement apr_table_t using
>>apr_hash_t" approach that I prefer.
>>
>
>That would make tables a hash again, which is what I disagree with. Yes,
>creating a new API will require module authors to modify their code to take
>advantage of the hash table performance. But, it keeps the table API refering
>to tables, instead of hashes. This will also require more modifications to
>httpd, but in the end, it keeps the code easier to read and understand.
>
To me, a table, even if implemented using apr_hash_t, is different from
an apr_hash_t. The apr_table_t, as it's been used in the httpd, isn't a
general-purpose set-of-strings. Rather, it's a set-of-strings-with-special-
support-for-concatenation-and-merging-that-works-well-for-http-headers.
Operations that are essential on tables, like apr_table_merge() and the
corresponding support in apr_table_overlap(), don't make sense for the
general-purpose, type-neutral apr_hash_t.
I'm also opposed to having a declaration like:
apr_hash_t *headers_in;
apr_hash_t *headers_out;
in the request_rec. Architecturally, those objects need to be unordered
sets with
fast access and the aforementioned merge support. They don't need to be
hash tables
specifically. The implementation could just as well be a balanced tree;
all the code
in the httpd that uses the httpd shouldn't know or care about how the
"unordered set"
object is implemented.
--Brian
Re: [PATCH] Turn apr_table_t into a hash table
Posted by Brian Pane <bp...@pacbell.net>.
Ryan Bloom wrote:
>On Saturday 08 September 2001 15:25, Brian Pane wrote:
>
>>Ryan Bloom wrote:
>>
>>>>>The latter. Having two API's to the same functions should only be done
>>>>>as a stop-gap.
>>>>>
>>>>I disagree. It's inevitable to have two APIs, as long as we have two
>>>>'table' types with very different semantics.
>>>>
>>>>apr_table_t is statically typed (uses char*), and apr_hash_t isn't (uses
>>>>void*).
>>>>If we did a direct replacement of apr_table_t with apr_hash_t in the
>>>>httpd, we'd be sacrificing static type-safety for a lot of code.
>>>>
>>>So create a simple set of prototypes and macros for a char * version of
>>>the hash code, the same way we did for ap_strchr and ap_strchr_c. If you
>>>do that, you will be adding probably two or three new functions to APR.\
>>>
>>Yes, macro wrappers would work. But if we use that approach, it would
>>be advantageous to use macro names like "apr_table_get" so that all
>>the existing httpd code and 3rd party modules can be used without
>>modification.
>>This would be functionally equivalent to the "implement apr_table_t using
>>apr_hash_t" approach that I prefer.
>>
>
>That would make tables a hash again, which is what I disagree with. Yes,
>creating a new API will require module authors to modify their code to take
>advantage of the hash table performance. But, it keeps the table API refering
>to tables, instead of hashes. This will also require more modifications to
>httpd, but in the end, it keeps the code easier to read and understand.
>
To me, a table, even if implemented using apr_hash_t, is different from
an apr_hash_t. The apr_table_t, as it's been used in the httpd, isn't a
general-purpose set-of-strings. Rather, it's a set-of-strings-with-special-
support-for-concatenation-and-merging-that-works-well-for-http-headers.
Operations that are essential on tables, like apr_table_merge() and the
corresponding support in apr_table_overlap(), don't make sense for the
general-purpose, type-neutral apr_hash_t.
I'm also opposed to having a declaration like:
apr_hash_t *headers_in;
apr_hash_t *headers_out;
in the request_rec. Architecturally, those objects need to be unordered
sets with
fast access and the aforementioned merge support. They don't need to be
hash tables
specifically. The implementation could just as well be a balanced tree;
all the code
in the httpd that uses the httpd shouldn't know or care about how the
"unordered set"
object is implemented.
--Brian
Re: [PATCH] Turn apr_table_t into a hash table
Posted by Ryan Bloom <rb...@covalent.net>.
On Saturday 08 September 2001 15:25, Brian Pane wrote:
> Ryan Bloom wrote:
> >>>The latter. Having two API's to the same functions should only be done
> >>>as a stop-gap.
> >>
> >>I disagree. It's inevitable to have two APIs, as long as we have two
> >>'table' types with very different semantics.
> >>
> >>apr_table_t is statically typed (uses char*), and apr_hash_t isn't (uses
> >>void*).
> >>If we did a direct replacement of apr_table_t with apr_hash_t in the
> >> httpd, we'd be sacrificing static type-safety for a lot of code.
> >
> >So create a simple set of prototypes and macros for a char * version of
> > the hash code, the same way we did for ap_strchr and ap_strchr_c. If you
> > do that, you will be adding probably two or three new functions to APR.\
>
> Yes, macro wrappers would work. But if we use that approach, it would
> be advantageous to use macro names like "apr_table_get" so that all
> the existing httpd code and 3rd party modules can be used without
> modification.
> This would be functionally equivalent to the "implement apr_table_t using
> apr_hash_t" approach that I prefer.
That would make tables a hash again, which is what I disagree with. Yes,
creating a new API will require module authors to modify their code to take
advantage of the hash table performance. But, it keeps the table API refering
to tables, instead of hashes. This will also require more modifications to
httpd, but in the end, it keeps the code easier to read and understand.
> > I don't know if any of them use this feature or not, but they aren't
> >a part of the core. Did you look at all of the modules in
> > modules.apache.org, or freshmeat? I know that there aren't many modules
> > for 2.0 today, but at some point, everybody who has a module for 1.3 will
> > want to port it to 2.0. I can currently do that in under one hour for
> > even complex modules. Changing API's like this after we have had 25
> > releases, makes that harder.
>
> Based on how infrequently apr_table_elts is used in the core and in PHP and
> mod_perl, and how small the code change required to use the iterator API
> is, I'd be extremely surprised if the removal of apr_table_elts
> substantially impaired one's ability to port a module from 1.3 to 2.0 in
> under an hour.
I personally have three modules that use ap_table_elts to iterate through
a table in Apache 1.3. I can port them to hashes if I have to, but the performance
impact doesn't concern me, because all I want to do with these modules, is
iterate over a small table.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: [PATCH] Turn apr_table_t into a hash table
Posted by Ryan Bloom <rb...@covalent.net>.
On Saturday 08 September 2001 15:25, Brian Pane wrote:
> Ryan Bloom wrote:
> >>>The latter. Having two API's to the same functions should only be done
> >>>as a stop-gap.
> >>
> >>I disagree. It's inevitable to have two APIs, as long as we have two
> >>'table' types with very different semantics.
> >>
> >>apr_table_t is statically typed (uses char*), and apr_hash_t isn't (uses
> >>void*).
> >>If we did a direct replacement of apr_table_t with apr_hash_t in the
> >> httpd, we'd be sacrificing static type-safety for a lot of code.
> >
> >So create a simple set of prototypes and macros for a char * version of
> > the hash code, the same way we did for ap_strchr and ap_strchr_c. If you
> > do that, you will be adding probably two or three new functions to APR.\
>
> Yes, macro wrappers would work. But if we use that approach, it would
> be advantageous to use macro names like "apr_table_get" so that all
> the existing httpd code and 3rd party modules can be used without
> modification.
> This would be functionally equivalent to the "implement apr_table_t using
> apr_hash_t" approach that I prefer.
That would make tables a hash again, which is what I disagree with. Yes,
creating a new API will require module authors to modify their code to take
advantage of the hash table performance. But, it keeps the table API refering
to tables, instead of hashes. This will also require more modifications to
httpd, but in the end, it keeps the code easier to read and understand.
> > I don't know if any of them use this feature or not, but they aren't
> >a part of the core. Did you look at all of the modules in
> > modules.apache.org, or freshmeat? I know that there aren't many modules
> > for 2.0 today, but at some point, everybody who has a module for 1.3 will
> > want to port it to 2.0. I can currently do that in under one hour for
> > even complex modules. Changing API's like this after we have had 25
> > releases, makes that harder.
>
> Based on how infrequently apr_table_elts is used in the core and in PHP and
> mod_perl, and how small the code change required to use the iterator API
> is, I'd be extremely surprised if the removal of apr_table_elts
> substantially impaired one's ability to port a module from 1.3 to 2.0 in
> under an hour.
I personally have three modules that use ap_table_elts to iterate through
a table in Apache 1.3. I can port them to hashes if I have to, but the performance
impact doesn't concern me, because all I want to do with these modules, is
iterate over a small table.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: [PATCH] Turn apr_table_t into a hash table
Posted by Brian Pane <bp...@pacbell.net>.
Ryan Bloom wrote:
>>>The latter. Having two API's to the same functions should only be done
>>>as a stop-gap.
>>>
>>I disagree. It's inevitable to have two APIs, as long as we have two
>>'table' types with very different semantics.
>>
>>apr_table_t is statically typed (uses char*), and apr_hash_t isn't (uses
>>void*).
>>If we did a direct replacement of apr_table_t with apr_hash_t in the httpd,
>>we'd be sacrificing static type-safety for a lot of code.
>>
>
>So create a simple set of prototypes and macros for a char * version of the
>hash code, the same way we did for ap_strchr and ap_strchr_c. If you do
>that, you will be adding probably two or three new functions to APR.\
>
Yes, macro wrappers would work. But if we use that approach, it would
be advantageous to use macro names like "apr_table_get" so that all
the existing httpd code and 3rd party modules can be used without
modification.
This would be functionally equivalent to the "implement apr_table_t using
apr_hash_t" approach that I prefer.
>>> Also, a LOT of modules using the apr_table_elem macro
>>>to get the elements from a table. Turning that into an API would break a
>>>lot of people.
>>>
>>My patch covers all the changes needed for the core modules. Do you have
>>any info on the number of 3rd-party modules for 2.0 that use
>>apr_table_elem?
>>
>
>The core is not the only set of modules for Apache. Did you look at mod_perl,
>php, tomcat?
>
mod_perl and Tomcat (mod_jk) don't use apr_table_elts. PHP contains two
calls to it.
> I don't know if any of them use this feature or not, but they aren't
>a part of the core. Did you look at all of the modules in modules.apache.org, or
>freshmeat? I know that there aren't many modules for 2.0 today, but at some
>point, everybody who has a module for 1.3 will want to port it to 2.0. I can
>currently do that in under one hour for even complex modules. Changing API's
>like this after we have had 25 releases, makes that harder.
>
Based on how infrequently apr_table_elts is used in the core and in PHP and
mod_perl, and how small the code change required to use the iterator API is,
I'd be extremely surprised if the removal of apr_table_elts substantially
impaired one's ability to port a module from 1.3 to 2.0 in under an hour.
--Brian
Re: [PATCH] Turn apr_table_t into a hash table
Posted by Brian Pane <bp...@pacbell.net>.
Ryan Bloom wrote:
>>>The latter. Having two API's to the same functions should only be done
>>>as a stop-gap.
>>>
>>I disagree. It's inevitable to have two APIs, as long as we have two
>>'table' types with very different semantics.
>>
>>apr_table_t is statically typed (uses char*), and apr_hash_t isn't (uses
>>void*).
>>If we did a direct replacement of apr_table_t with apr_hash_t in the httpd,
>>we'd be sacrificing static type-safety for a lot of code.
>>
>
>So create a simple set of prototypes and macros for a char * version of the
>hash code, the same way we did for ap_strchr and ap_strchr_c. If you do
>that, you will be adding probably two or three new functions to APR.\
>
Yes, macro wrappers would work. But if we use that approach, it would
be advantageous to use macro names like "apr_table_get" so that all
the existing httpd code and 3rd party modules can be used without
modification.
This would be functionally equivalent to the "implement apr_table_t using
apr_hash_t" approach that I prefer.
>>> Also, a LOT of modules using the apr_table_elem macro
>>>to get the elements from a table. Turning that into an API would break a
>>>lot of people.
>>>
>>My patch covers all the changes needed for the core modules. Do you have
>>any info on the number of 3rd-party modules for 2.0 that use
>>apr_table_elem?
>>
>
>The core is not the only set of modules for Apache. Did you look at mod_perl,
>php, tomcat?
>
mod_perl and Tomcat (mod_jk) don't use apr_table_elts. PHP contains two
calls to it.
> I don't know if any of them use this feature or not, but they aren't
>a part of the core. Did you look at all of the modules in modules.apache.org, or
>freshmeat? I know that there aren't many modules for 2.0 today, but at some
>point, everybody who has a module for 1.3 will want to port it to 2.0. I can
>currently do that in under one hour for even complex modules. Changing API's
>like this after we have had 25 releases, makes that harder.
>
Based on how infrequently apr_table_elts is used in the core and in PHP and
mod_perl, and how small the code change required to use the iterator API is,
I'd be extremely surprised if the removal of apr_table_elts substantially
impaired one's ability to port a module from 1.3 to 2.0 in under an hour.
--Brian
Re: [PATCH] Turn apr_table_t into a hash table
Posted by Ryan Bloom <rb...@covalent.net>.
> >The latter. Having two API's to the same functions should only be done
> >as a stop-gap.
>
> I disagree. It's inevitable to have two APIs, as long as we have two
> 'table' types with very different semantics.
>
> apr_table_t is statically typed (uses char*), and apr_hash_t isn't (uses
> void*).
> If we did a direct replacement of apr_table_t with apr_hash_t in the httpd,
> we'd be sacrificing static type-safety for a lot of code.
So create a simple set of prototypes and macros for a char * version of the
hash code, the same way we did for ap_strchr and ap_strchr_c. If you do
that, you will be adding probably two or three new functions to APR.
> > Also, a LOT of modules using the apr_table_elem macro
> >to get the elements from a table. Turning that into an API would break a
> >lot of people.
>
> My patch covers all the changes needed for the core modules. Do you have
> any info on the number of 3rd-party modules for 2.0 that use
> apr_table_elem?
The core is not the only set of modules for Apache. Did you look at mod_perl,
php, tomcat? I don't know if any of them use this feature or not, but they aren't
a part of the core. Did you look at all of the modules in modules.apache.org, or
freshmeat? I know that there aren't many modules for 2.0 today, but at some
point, everybody who has a module for 1.3 will want to port it to 2.0. I can
currently do that in under one hour for even complex modules. Changing API's
like this after we have had 25 releases, makes that harder.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: [PATCH] Turn apr_table_t into a hash table
Posted by Ryan Bloom <rb...@covalent.net>.
> >The latter. Having two API's to the same functions should only be done
> >as a stop-gap.
>
> I disagree. It's inevitable to have two APIs, as long as we have two
> 'table' types with very different semantics.
>
> apr_table_t is statically typed (uses char*), and apr_hash_t isn't (uses
> void*).
> If we did a direct replacement of apr_table_t with apr_hash_t in the httpd,
> we'd be sacrificing static type-safety for a lot of code.
So create a simple set of prototypes and macros for a char * version of the
hash code, the same way we did for ap_strchr and ap_strchr_c. If you do
that, you will be adding probably two or three new functions to APR.
> > Also, a LOT of modules using the apr_table_elem macro
> >to get the elements from a table. Turning that into an API would break a
> >lot of people.
>
> My patch covers all the changes needed for the core modules. Do you have
> any info on the number of 3rd-party modules for 2.0 that use
> apr_table_elem?
The core is not the only set of modules for Apache. Did you look at mod_perl,
php, tomcat? I don't know if any of them use this feature or not, but they aren't
a part of the core. Did you look at all of the modules in modules.apache.org, or
freshmeat? I know that there aren't many modules for 2.0 today, but at some
point, everybody who has a module for 1.3 will want to port it to 2.0. I can
currently do that in under one hour for even complex modules. Changing API's
like this after we have had 25 releases, makes that harder.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: [PATCH] Turn apr_table_t into a hash table
Posted by Brian Pane <bp...@pacbell.net>.
Ryan Bloom wrote:
>On Saturday 08 September 2001 11:29, Brian Pane wrote:
>
>>Ryan Bloom wrote:
>>
>>>On Friday 07 September 2001 14:23, Brian Pane wrote:
>>>
>>>>The attached patches change the apr_table_t implementation from
>>>>a linear list to a hash table (not an apr_hash_t, though!). With
>>>>this change, I'm seeing a ~3% improvement in throughput when
>>>>delivering a 0-byte file over the loopback on Linux. (I used this
>>>>0-byte test case to measure the inherent overhead in the httpd, without
>>>>transmission time clouding the results.)
>>>>
>>>I dislike this. Why are we putting a second hash table into APR? If we
>>>want to use a hash, then ues an apr_hash_t. If apr_hash_t doesn't
>>>support something that we MUST have to do this, then fix apr_hash_t.
>>>Having two different hash alorithms in APR, one of them hidden under a
>>>tables API, seems kind of hackish to me.
>>>
>>Are you arguing in favor of using apr_hash_t in the implementation of
>>apr_table_t,
>>or using apr_hash_t in place of apr_table_t in the request_rec? I'm
>>comfortable
>>with the former, but not the latter.
>>
>
>The latter. Having two API's to the same functions should only be done
>as a stop-gap.
>
I disagree. It's inevitable to have two APIs, as long as we have two
'table' types with very different semantics.
apr_table_t is statically typed (uses char*), and apr_hash_t isn't (uses
void*).
If we did a direct replacement of apr_table_t with apr_hash_t in the httpd,
we'd be sacrificing static type-safety for a lot of code.
> Also, a LOT of modules using the apr_table_elem macro
>to get the elements from a table. Turning that into an API would break a
>lot of people.
>
My patch covers all the changes needed for the core modules. Do you have
any info on the number of 3rd-party modules for 2.0 that use apr_table_elem?
--Brian
Re: [PATCH] Turn apr_table_t into a hash table
Posted by Brian Pane <bp...@pacbell.net>.
Ryan Bloom wrote:
>On Saturday 08 September 2001 11:29, Brian Pane wrote:
>
>>Ryan Bloom wrote:
>>
>>>On Friday 07 September 2001 14:23, Brian Pane wrote:
>>>
>>>>The attached patches change the apr_table_t implementation from
>>>>a linear list to a hash table (not an apr_hash_t, though!). With
>>>>this change, I'm seeing a ~3% improvement in throughput when
>>>>delivering a 0-byte file over the loopback on Linux. (I used this
>>>>0-byte test case to measure the inherent overhead in the httpd, without
>>>>transmission time clouding the results.)
>>>>
>>>I dislike this. Why are we putting a second hash table into APR? If we
>>>want to use a hash, then ues an apr_hash_t. If apr_hash_t doesn't
>>>support something that we MUST have to do this, then fix apr_hash_t.
>>>Having two different hash alorithms in APR, one of them hidden under a
>>>tables API, seems kind of hackish to me.
>>>
>>Are you arguing in favor of using apr_hash_t in the implementation of
>>apr_table_t,
>>or using apr_hash_t in place of apr_table_t in the request_rec? I'm
>>comfortable
>>with the former, but not the latter.
>>
>
>The latter. Having two API's to the same functions should only be done
>as a stop-gap.
>
I disagree. It's inevitable to have two APIs, as long as we have two
'table' types with very different semantics.
apr_table_t is statically typed (uses char*), and apr_hash_t isn't (uses
void*).
If we did a direct replacement of apr_table_t with apr_hash_t in the httpd,
we'd be sacrificing static type-safety for a lot of code.
> Also, a LOT of modules using the apr_table_elem macro
>to get the elements from a table. Turning that into an API would break a
>lot of people.
>
My patch covers all the changes needed for the core modules. Do you have
any info on the number of 3rd-party modules for 2.0 that use apr_table_elem?
--Brian
Re: [PATCH] Turn apr_table_t into a hash table
Posted by Ryan Bloom <rb...@covalent.net>.
On Saturday 08 September 2001 11:29, Brian Pane wrote:
> Ryan Bloom wrote:
> >On Friday 07 September 2001 14:23, Brian Pane wrote:
> >>The attached patches change the apr_table_t implementation from
> >>a linear list to a hash table (not an apr_hash_t, though!). With
> >>this change, I'm seeing a ~3% improvement in throughput when
> >>delivering a 0-byte file over the loopback on Linux. (I used this
> >>0-byte test case to measure the inherent overhead in the httpd, without
> >>transmission time clouding the results.)
> >
> >I dislike this. Why are we putting a second hash table into APR? If we
> > want to use a hash, then ues an apr_hash_t. If apr_hash_t doesn't
> > support something that we MUST have to do this, then fix apr_hash_t.
> > Having two different hash alorithms in APR, one of them hidden under a
> > tables API, seems kind of hackish to me.
>
> Are you arguing in favor of using apr_hash_t in the implementation of
> apr_table_t,
> or using apr_hash_t in place of apr_table_t in the request_rec? I'm
> comfortable
> with the former, but not the latter.
The latter. Having two API's to the same functions should only be done
as a stop-gap. Also, a LOT of modules using the apr_table_elem macro
to get the elements from a table. Turning that into an API would break a
lot of people.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: [PATCH] Turn apr_table_t into a hash table
Posted by Ryan Bloom <rb...@covalent.net>.
On Saturday 08 September 2001 11:29, Brian Pane wrote:
> Ryan Bloom wrote:
> >On Friday 07 September 2001 14:23, Brian Pane wrote:
> >>The attached patches change the apr_table_t implementation from
> >>a linear list to a hash table (not an apr_hash_t, though!). With
> >>this change, I'm seeing a ~3% improvement in throughput when
> >>delivering a 0-byte file over the loopback on Linux. (I used this
> >>0-byte test case to measure the inherent overhead in the httpd, without
> >>transmission time clouding the results.)
> >
> >I dislike this. Why are we putting a second hash table into APR? If we
> > want to use a hash, then ues an apr_hash_t. If apr_hash_t doesn't
> > support something that we MUST have to do this, then fix apr_hash_t.
> > Having two different hash alorithms in APR, one of them hidden under a
> > tables API, seems kind of hackish to me.
>
> Are you arguing in favor of using apr_hash_t in the implementation of
> apr_table_t,
> or using apr_hash_t in place of apr_table_t in the request_rec? I'm
> comfortable
> with the former, but not the latter.
The latter. Having two API's to the same functions should only be done
as a stop-gap. Also, a LOT of modules using the apr_table_elem macro
to get the elements from a table. Turning that into an API would break a
lot of people.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: [PATCH] Turn apr_table_t into a hash table
Posted by Brian Pane <bp...@pacbell.net>.
Ryan Bloom wrote:
>On Friday 07 September 2001 14:23, Brian Pane wrote:
>
>>The attached patches change the apr_table_t implementation from
>>a linear list to a hash table (not an apr_hash_t, though!). With
>>this change, I'm seeing a ~3% improvement in throughput when
>>delivering a 0-byte file over the loopback on Linux. (I used this
>>0-byte test case to measure the inherent overhead in the httpd, without
>>transmission time clouding the results.)
>>
>
>I dislike this. Why are we putting a second hash table into APR? If we want
>to use a hash, then ues an apr_hash_t. If apr_hash_t doesn't support something
>that we MUST have to do this, then fix apr_hash_t. Having two different hash
>alorithms in APR, one of them hidden under a tables API, seems kind of hackish
>to me.
>
Are you arguing in favor of using apr_hash_t in the implementation of
apr_table_t,
or using apr_hash_t in place of apr_table_t in the request_rec? I'm
comfortable
with the former, but not the latter.
--Brian
Re: [PATCH] Turn apr_table_t into a hash table
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
From: "Ryan Bloom" <rb...@covalent.net>
Sent: Saturday, September 08, 2001 8:32 AM
> On Friday 07 September 2001 14:23, Brian Pane wrote:
> > The attached patches change the apr_table_t implementation from
> > a linear list to a hash table (not an apr_hash_t, though!). With
> > this change, I'm seeing a ~3% improvement in throughput when
> > delivering a 0-byte file over the loopback on Linux. (I used this
> > 0-byte test case to measure the inherent overhead in the httpd, without
> > transmission time clouding the results.)
>
> I dislike this. Why are we putting a second hash table into APR? If we want
> to use a hash, then ues an apr_hash_t. If apr_hash_t doesn't support something
> that we MUST have to do this, then fix apr_hash_t. Having two different hash
> alorithms in APR, one of them hidden under a tables API, seems kind of hackish
> to me.
Agreed. Because of the ordering issues (with insertion order) that I brought up
earlier, this is probably not the best place to introduce such a feature.
apr_hash_t should be sufficient, and if it is not, please suggest improvements
to the apr_hash_t api :)
Bill
Re: [PATCH] Turn apr_table_t into a hash table
Posted by Brian Pane <bp...@pacbell.net>.
Ryan Bloom wrote:
>On Friday 07 September 2001 14:23, Brian Pane wrote:
>
>>The attached patches change the apr_table_t implementation from
>>a linear list to a hash table (not an apr_hash_t, though!). With
>>this change, I'm seeing a ~3% improvement in throughput when
>>delivering a 0-byte file over the loopback on Linux. (I used this
>>0-byte test case to measure the inherent overhead in the httpd, without
>>transmission time clouding the results.)
>>
>
>I dislike this. Why are we putting a second hash table into APR? If we want
>to use a hash, then ues an apr_hash_t. If apr_hash_t doesn't support something
>that we MUST have to do this, then fix apr_hash_t. Having two different hash
>alorithms in APR, one of them hidden under a tables API, seems kind of hackish
>to me.
>
Are you arguing in favor of using apr_hash_t in the implementation of
apr_table_t,
or using apr_hash_t in place of apr_table_t in the request_rec? I'm
comfortable
with the former, but not the latter.
--Brian
Re: [PATCH] Turn apr_table_t into a hash table
Posted by Ryan Bloom <rb...@covalent.net>.
On Friday 07 September 2001 14:23, Brian Pane wrote:
> The attached patches change the apr_table_t implementation from
> a linear list to a hash table (not an apr_hash_t, though!). With
> this change, I'm seeing a ~3% improvement in throughput when
> delivering a 0-byte file over the loopback on Linux. (I used this
> 0-byte test case to measure the inherent overhead in the httpd, without
> transmission time clouding the results.)
I dislike this. Why are we putting a second hash table into APR? If we want
to use a hash, then ues an apr_hash_t. If apr_hash_t doesn't support something
that we MUST have to do this, then fix apr_hash_t. Having two different hash
alorithms in APR, one of them hidden under a tables API, seems kind of hackish
to me.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: [PATCH] Turn apr_table_t into a hash table
Posted by Ryan Bloom <rb...@covalent.net>.
On Friday 07 September 2001 14:23, Brian Pane wrote:
> The attached patches change the apr_table_t implementation from
> a linear list to a hash table (not an apr_hash_t, though!). With
> this change, I'm seeing a ~3% improvement in throughput when
> delivering a 0-byte file over the loopback on Linux. (I used this
> 0-byte test case to measure the inherent overhead in the httpd, without
> transmission time clouding the results.)
I dislike this. Why are we putting a second hash table into APR? If we want
to use a hash, then ues an apr_hash_t. If apr_hash_t doesn't support something
that we MUST have to do this, then fix apr_hash_t. Having two different hash
alorithms in APR, one of them hidden under a tables API, seems kind of hackish
to me.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------