You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Chris Darroch <ch...@pearsoncmg.com> on 2006/05/04 19:17:45 UTC
[PATCH 1/6] scoreboard over-sized
Hi --
It looks to me like the memory allocated for ap_scoreboard_image
is a little bit over-sized. In r104404 the lb_score elements were
added to the scoreboard in the manner of the worker_score array,
and then in r105134 much of this was reversed, but the call to
calloc() still sizes ap_scoreboard_image as if a two-dimensional
array was required.
Chris.
=====================================================================
--- server/scoreboard.c.orig 2006-05-02 09:52:09.803650679 -0400
+++ server/scoreboard.c 2006-05-03 10:17:13.273161088 -0400
@@ -117,8 +117,7 @@
ap_calc_scoreboard_size();
ap_scoreboard_image =
- calloc(1, sizeof(scoreboard) + server_limit * sizeof(worker_score *) +
- server_limit * lb_limit * sizeof(lb_score *));
+ calloc(1, sizeof(scoreboard) + server_limit * sizeof(worker_score *));
more_storage = shared_score;
ap_scoreboard_image->global = (global_score *)more_storage;
more_storage += sizeof(global_score);
Re: [PATCH 1/6] scoreboard over-sized
Posted by Colm MacCarthaigh <co...@stdlib.net>.
On Fri, May 05, 2006 at 11:20:14PM +0100, Nick Kew wrote:
> Looks right for ap_init_scoreboard, and there's nothing else relevant
> in scoreboard.c. A quick grep suggests it's globally right, so +1.
Same here, afaict the patch gets it right.
--
Colm MacCárthaigh Public Key: colm+pgp@stdlib.net
Re: [PATCH 1/6] scoreboard over-sized
Posted by Nick Kew <ni...@webthing.com>.
On Thursday 04 May 2006 18:17, Chris Darroch wrote:
> Hi --
>
> It looks to me like the memory allocated for ap_scoreboard_image
> is a little bit over-sized. In r104404 the lb_score elements were
> added to the scoreboard in the manner of the worker_score array,
> and then in r105134 much of this was reversed, but the call to
> calloc() still sizes ap_scoreboard_image as if a two-dimensional
> array was required.
>
> Chris.
>
> =====================================================================
> --- server/scoreboard.c.orig 2006-05-02 09:52:09.803650679 -0400
> +++ server/scoreboard.c 2006-05-03 10:17:13.273161088 -0400
> @@ -117,8 +117,7 @@
>
> ap_calc_scoreboard_size();
> ap_scoreboard_image =
> - calloc(1, sizeof(scoreboard) + server_limit * sizeof(worker_score
> *) + - server_limit * lb_limit * sizeof(lb_score *));
> + calloc(1, sizeof(scoreboard) + server_limit * sizeof(worker_score
> *)); more_storage = shared_score;
> ap_scoreboard_image->global = (global_score *)more_storage;
> more_storage += sizeof(global_score);
Looks right for ap_init_scoreboard, and there's nothing else relevant
in scoreboard.c. A quick grep suggests it's globally right, so +1.
Your patch 2: an easier +1.
Patch 3 - again +1; colm thinks it unnecessary, but it's at worst harmless.
Patches 4/5: bug me if I don't get around to reviewing them over the weekend.
Nice work on some of apache's hairiest code!
--
Nick Kew
Re: [PATCH 1/6] scoreboard over-sized
Posted by Jean-frederic Clere <jf...@gmail.com>.
Chris Darroch wrote:
>Jean-frederic Clere wrote:
>
>
>
>>BTW: lb_scrore has a size of 1024 and proxy_worker_stat 176...
>>I am thinking in using this not yet used space.
>>
>>
>
> You're referring to ap_proxy_initialize_worker_share() in
>proxy_util.c, I take it? It seems to be the only user of the
>1024 byte array in lb_score and fills it with proxy_worker_stat
>structures.
>
>
Yes that is what I am refering to.
> For a private module, you could of course use the extra space;
>it's your call. The comments in scoreboard.h about the 1024 byte
>array ("TODO: make a real stuct from this") make it fairly clear
>that you shouldn't assume that array is a permanent feature, though.
>
> Personally, if I was writing a completely private module, I'd
>probably just patch the scoreboard.h definition of lb_score to include
>whatever extra stuff I needed, and recompile httpd. That way I'm
>insulated from any changes to the size of either the array or the
>proxy_worker_stat structure; if they change at some point in a
>future release and I don't happen to notice, I'm safe so long as
>I keep patching in my extra fields.
>
>
Ok.
> Now, if you're thinking instead of contributing a public patch
>to httpd, then I think that you'd definitely want to avoid just
>using more of the array -- it's a small hack now, but adding to
>it will only make it harder to tidy up later. Better to add
>some fields to lb_score so that people can understand what's going
>on with your stuff. That's just IMHO, of course.
>
>Chris.
>
>
>
Re: [PATCH 1/6] scoreboard over-sized
Posted by Chris Darroch <ch...@pearsoncmg.com>.
Jean-frederic Clere wrote:
> BTW: lb_scrore has a size of 1024 and proxy_worker_stat 176...
> I am thinking in using this not yet used space.
You're referring to ap_proxy_initialize_worker_share() in
proxy_util.c, I take it? It seems to be the only user of the
1024 byte array in lb_score and fills it with proxy_worker_stat
structures.
For a private module, you could of course use the extra space;
it's your call. The comments in scoreboard.h about the 1024 byte
array ("TODO: make a real stuct from this") make it fairly clear
that you shouldn't assume that array is a permanent feature, though.
Personally, if I was writing a completely private module, I'd
probably just patch the scoreboard.h definition of lb_score to include
whatever extra stuff I needed, and recompile httpd. That way I'm
insulated from any changes to the size of either the array or the
proxy_worker_stat structure; if they change at some point in a
future release and I don't happen to notice, I'm safe so long as
I keep patching in my extra fields.
Now, if you're thinking instead of contributing a public patch
to httpd, then I think that you'd definitely want to avoid just
using more of the array -- it's a small hack now, but adding to
it will only make it harder to tidy up later. Better to add
some fields to lb_score so that people can understand what's going
on with your stuff. That's just IMHO, of course.
Chris.
--
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
Re: [PATCH 1/6] scoreboard over-sized
Posted by Jean-frederic Clere <jf...@gmail.com>.
Chris Darroch wrote:
>Hi --
>
> It looks to me like the memory allocated for ap_scoreboard_image
>is a little bit over-sized. In r104404 the lb_score elements were
>added to the scoreboard in the manner of the worker_score array,
>
>
BTW: lb_scrore has a size of 1024 and proxy_worker_stat 176...
I am thinking in using this not yet used space.
Any comments?
Cheers
Jean-Frederic
>and then in r105134 much of this was reversed, but the call to
>calloc() still sizes ap_scoreboard_image as if a two-dimensional
>array was required.
>
>Chris.
>
>=====================================================================
>--- server/scoreboard.c.orig 2006-05-02 09:52:09.803650679 -0400
>+++ server/scoreboard.c 2006-05-03 10:17:13.273161088 -0400
>@@ -117,8 +117,7 @@
>
> ap_calc_scoreboard_size();
> ap_scoreboard_image =
>- calloc(1, sizeof(scoreboard) + server_limit * sizeof(worker_score *) +
>- server_limit * lb_limit * sizeof(lb_score *));
>+ calloc(1, sizeof(scoreboard) + server_limit * sizeof(worker_score *));
> more_storage = shared_score;
> ap_scoreboard_image->global = (global_score *)more_storage;
> more_storage += sizeof(global_score);
>
>
>
>