You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2012/01/16 14:11:57 UTC
Re: svn commit: r1231605 - /apr/apr/trunk/tables/apr_hash.c
Bojan Smojver wrote:
> On Sun, 2012-01-15 at 18:06 +0100, Bert Huijben wrote:
>> If the timer has enough detail we could just use the time, ptr
>> combination as the seed here.
>
> See whether you like r1231858.
>
r1231605 and r1231858 cause massive regressions and test case failures in httpd.
Not sure why right now.
est Summary Report
-------------------
t/apache/acceptpathinfo.t (Wstat: 0 Tests: 36 Failed: 10)
Failed tests: 2, 6, 8, 12, 14, 18, 26, 30, 35-36
t/apache/cfg_getline.t (Wstat: 0 Tests: 116 Failed: 58)
Failed tests: 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22
24, 26, 28, 30, 32, 34, 36, 38, 40, 42
44, 46, 48, 50, 52, 54, 56, 58, 60, 62
64, 66, 68, 70, 72, 74, 76, 78, 80, 82
84, 86, 88, 90, 92, 94, 96, 98, 100, 102
104, 106, 108, 110, 112, 114, 116
t/apache/pr17629.t (Wstat: 0 Tests: 4 Failed: 2)
Failed tests: 1, 3
t/apache/pr43939.t (Wstat: 0 Tests: 4 Failed: 2)
Failed tests: 1, 3
t/apache/pr49328.t (Wstat: 0 Tests: 1 Failed: 1)
Failed test: 1
t/modules/asis.t (Wstat: 0 Tests: 3 Failed: 3)
Failed tests: 1-3
t/modules/filter.t (Wstat: 0 Tests: 1 Failed: 1)
Failed test: 1
t/modules/lua.t (Wstat: 0 Tests: 36 Failed: 8)
Failed tests: 7-8, 13-14, 16-17, 22-23
t/modules/negotiation.t (Wstat: 0 Tests: 99 Failed: 1)
Failed test: 99
t/modules/proxy.t (Wstat: 0 Tests: 15 Failed: 2)
Failed tests: 9-10
Files=104, Tests=4551, 112 wallclock secs ( 1.37 usr 0.53 sys + 37.48 cusr 14.01 csys = 53.39 CPU)
Regards
Rüdiger
Re: svn commit: r1231605 - /apr/apr/trunk/tables/apr_hash.c
Posted by Bojan Smojver <bo...@rexursive.com>.
------- Original message -------
> From: Bojan Smojver
> Sent: 17.1.'12, 5:18
>
> ------- Original message -------
>> From: Ruediger Pluem
>
>> r1231605 and r1231858 cause massive regressions and test case failures
>> in httpd.
>
> I won't be able to commit for a while. Please feel free to revert both.
> Sorry about the breakage. :-(
I had a look at the apr_hash.c and I see that I completely missed patching
the merge function. So, if the tests in question are using it, they will
fail for sure.
--
Bojan
Re: svn commit: r1231605 - /apr/apr/trunk/tables/apr_hash.c
Posted by Bojan Smojver <bo...@rexursive.com>.
------- Original message -------
> From: Bojan Smojver
> Just reverted both.
I'll work on that merge functionality that I missed and will actually test
more before comitting anything. Not near a decent computer now, so it may
be a while.
--
Bojan
Re: svn commit: r1231605 - /apr/apr/trunk/tables/apr_hash.c
Posted by Bojan Smojver <bo...@rexursive.com>.
------- Original message -------
> From: Bojan Smojver
> Sent: 17.1.'12, 5:18
>
> ------- Original message -------
>> From: Ruediger Pluem
>
>> r1231605 and r1231858 cause massive regressions and test case failures
>> in httpd.
>
> I won't be able to commit for a while. Please feel free to revert both.
> Sorry about the breakage. :-(
Just reverted both.
--
Bojan
Re: svn commit: r1231605 - /apr/apr/trunk/tables/apr_hash.c
Posted by Bojan Smojver <bo...@rexursive.com>.
------- Original message -------
> From: Ruediger Pluem
> r1231605 and r1231858 cause massive regressions and test case failures in
> httpd.
I won't be able to commit for a while. Please feel free to revert both.
Sorry about the breakage. :-(
--
Bojan
Re: svn commit: r1231605 - /apr/apr/trunk/tables/apr_hash.c
Posted by Ruediger Pluem <rp...@apache.org>.
Bojan Smojver wrote:
> On Thu, 2012-01-26 at 09:05 +1100, Bojan Smojver wrote:
>> Will fix.
>
> Better?
>
Yes. No more regression in httpd and APR tests pass as well.
Regards
Rüdiger
Re: svn commit: r1231605 - /apr/apr/trunk/tables/apr_hash.c
Posted by Bojan Smojver <bo...@rexursive.com>.
On Thu, 2012-01-26 at 09:05 +1100, Bojan Smojver wrote:
> Will fix.
Better?
--
Bojan
Re: svn commit: r1231605 - /apr/apr/trunk/tables/apr_hash.c
Posted by Bojan Smojver <bo...@rexursive.com>.
------- Original message -------
> From: Ruediger Pluem
> Shouldn't you store the result of res->hash_func /
> apr_hashfunc_default_internal in a local temporary
> variable and use it later on? Otherwise you change the overlay hash and
> may make it unusable by setting a new hash
> value. IMHO all operations by this code are currently readonly on the
> base and overlay hash.
Yeah, good point. I was working under the assumption that overlay gets
thrown away, which may not be true. Will fix.
--
Bojan
Re: svn commit: r1231605 - /apr/apr/trunk/tables/apr_hash.c
Posted by Ruediger Pluem <rp...@apache.org>.
Bojan Smojver wrote:
> On Mon, 2012-01-16 at 14:11 +0100, Ruediger Pluem wrote:
>> r1231605 and r1231858 cause massive regressions and test case failures
>> in httpd. Not sure why right now.
>
> Could you please run your tests with this patch. Let me know how it
> goes. Thanks.
>
I think there is still a problem with your patch:
@@ -468,6 +484,12 @@
for (k = 0; k <= overlay->max; k++) {
for (iter = overlay->array[k]; iter; iter = iter->next) {
+ if (res->hash_func)
+ iter->hash = res->hash_func(iter->key, &iter->klen);
+ else
+ iter->hash = apr_hashfunc_default_internal(iter->key,
+ &iter->klen,
+ res->seed);
Shouldn't you store the result of res->hash_func / apr_hashfunc_default_internal in a local temporary
variable and use it later on? Otherwise you change the overlay hash and may make it unusable by setting a new hash
value. IMHO all operations by this code are currently readonly on the base and overlay hash.
i = iter->hash & res->max;
for (ent = res->array[i]; ent; ent = ent->next) {
if ((ent->klen == iter->klen) &&
Regards
Rüdiger
Re: svn commit: r1231605 - /apr/apr/trunk/tables/apr_hash.c
Posted by Bojan Smojver <bo...@rexursive.com>.
On Mon, 2012-01-16 at 14:11 +0100, Ruediger Pluem wrote:
> r1231605 and r1231858 cause massive regressions and test case failures
> in httpd. Not sure why right now.
Could you please run your tests with this patch. Let me know how it
goes. Thanks.
--
Bojan