Align worker's worker's fdqueue with event's?

[Yann Ylavic <yl...@gmail.com>]

Hi,

there a several optimizations and correctness fixes in event/fdqueue.c
that don't land in worker/fdqueue.c.

The patch would look like the attached.
It also include some cosmectic changes in event (mainly s/type *
arg/type *arg/) ala worker which suits more with the style used
elsewhere in httpd.

Thoughts?

For now things that are event only are not aligned (e.g. timers), but
ultimately I'd like to have a single fdqueue.[ch] for both MPMs (not
too far once this patch is applied), that'd certainly help maintenance
and improvements for both.
If you agree on this, what would be the best practice/place for the common code?


Regards,
Yann.

Re: Align worker's worker's fdqueue with event's?

[Eric Covener <co...@gmail.com>]

> I don't mean to make it API, still a private (unix specific/common)
> thing, something like "os/unix/unixd.c"'s non-AP_DECLARE things.

seems like there should be no big surprises here with analogs like
unixd, mpm_common, etc

Re: Align worker's worker's fdqueue with event's?

[Yann Ylavic <yl...@gmail.com>]

On Thu, Jan 11, 2018 at 1:34 PM, Stefan Eissing
<st...@greenbytes.de> wrote:
>> Am 11.01.2018 um 13:02 schrieb Yann Ylavic <yl...@gmail.com>:
>>
>> there a several optimizations and correctness fixes in event/fdqueue.c
>> that don't land in worker/fdqueue.c.
[]
> If we had a single, nice build system, I'd be all for it. With the
> current state of things, I am not sure it is worth our time to
> think about common code that is not in the public API,

At least that'd avoid missing fixes/improvements made in one code and
not the other.
Not sure fixing twice or more isn't what can consume our time too.

>
> Making both files the same, might still be worth it since future
> enhancements can be copied/patched more easily. And there are not
> *that* many people working in this area.

Which also suggests code should be centralized IMO, because new comers
may not know about the duplication and the need to fix in multiple
places.

>
>> For now things that are event only are not aligned (e.g. timers), but
>> ultimately I'd like to have a single fdqueue.[ch] for both MPMs (not
>> too far once this patch is applied), that'd certainly help maintenance
>> and improvements for both.
>> If you agree on this, what would be the best practice/place for the common code?
>
> Making things part of the API has a high price in back porting costs and
> not being able to take things away again.

I don't mean to make it API, still a private (unix specific/common)
thing, something like "os/unix/unixd.c"'s non-AP_DECLARE things.

>
> Related issues exist in modules too. mod_http2 and mod_proxy_http2 would
> like to share. mod_md once had an executable with shared code.
>
> I eliminated the sharing and the executable rather than having to deal
> with that pain. Not ideal, but there is limited time to put into such
> things.

Yeah, sharing from/between modules (APR_OPTIONAL) is another beast I'd
like to avoid here for 2 "unix" MPMs, hence the core non-AP_DECLARE
(hidden) proposal maybe.


Thanks,
Yann.

Re: Align worker's worker's fdqueue with event's?

[Stefan Eissing <st...@greenbytes.de>]

> Am 11.01.2018 um 13:02 schrieb Yann Ylavic <yl...@gmail.com>:
> 
> Hi,
> 
> there a several optimizations and correctness fixes in event/fdqueue.c
> that don't land in worker/fdqueue.c.
> 
> The patch would look like the attached.
> It also include some cosmectic changes in event (mainly s/type *
> arg/type *arg/) ala worker which suits more with the style used
> elsewhere in httpd.
> 
> Thoughts?

If we had a single, nice build system, I'd be all for it. With the
current state of things, I am not sure it is worth our time to
think about common code that is not in the public API,

Making both files the same, might still be worth it since future
enhancements can be copied/patched more easily. And there are not
*that* many people working in this area.

> For now things that are event only are not aligned (e.g. timers), but
> ultimately I'd like to have a single fdqueue.[ch] for both MPMs (not
> too far once this patch is applied), that'd certainly help maintenance
> and improvements for both.
> If you agree on this, what would be the best practice/place for the common code?

Making things part of the API has a high price in back porting costs and 
not being able to take things away again. 

Related issues exist in modules too. mod_http2 and mod_proxy_http2 would
like to share. mod_md once had an executable with shared code.

I eliminated the sharing and the executable rather than having to deal 
with that pain. Not ideal, but there is limited time to put into such
things.


Cheers,

Stefan

Re: Align worker's worker's fdqueue with event's?

[Luca Toscano <to...@gmail.com>]

2018-01-12 13:34 GMT+01:00 Ruediger Pluem <rp...@apache.org>:

>
>
> On 01/12/2018 01:32 PM, Eric Covener wrote:
> > On Fri, Jan 12, 2018 at 6:51 AM, Yann Ylavic <yl...@gmail.com>
> wrote:
> >> A bit orthogonal, I'd also like to sync 2.4.x "event" with trunk's
> >> w.r.t. cosmetic changes before (and to help) further backport
> >> proposals.
> >>
> >> That's possibly something that'll help *us* for later backports, but
> >> not necessarily distros with (security-)fixes only policy.
> >> Is that something we should more care about? I suppose distro
> >> maintainers do care...
> >>
> >> For instance, the three attached patches are how I would stage latest
> >> "event" changes in 2.4.x:
> >> - patch 1: align with trunk what can/needs to be (cosmetics);
> >> - patch 2: optimizations and correctness which don't seem to have
> >> bitten us so far (not a proven fix someow);
> >> - patch 3: a wakeup fix (corner case) that applies almost cleanly
> >> thanks to 1/ and 2/.
> >>
> >> Would this work or should I go with 3/ directly and resolve backport
> >> conflicts there?
> >> Or maybe go with 3/ then 2/ then 1/, for the same result but at least
> >> distros would care of the first step only (for this time...)?
> >
> > Looks reasonable to me, better to rip the band-aid off then pay the
> > price/risk every time something needs to be backported.
> >
>
> +1
>
>
+1!

Re: Align worker's worker's fdqueue with event's?

[Ruediger Pluem <rp...@apache.org>]

On 01/12/2018 01:32 PM, Eric Covener wrote:
> On Fri, Jan 12, 2018 at 6:51 AM, Yann Ylavic <yl...@gmail.com> wrote:
>> A bit orthogonal, I'd also like to sync 2.4.x "event" with trunk's
>> w.r.t. cosmetic changes before (and to help) further backport
>> proposals.
>>
>> That's possibly something that'll help *us* for later backports, but
>> not necessarily distros with (security-)fixes only policy.
>> Is that something we should more care about? I suppose distro
>> maintainers do care...
>>
>> For instance, the three attached patches are how I would stage latest
>> "event" changes in 2.4.x:
>> - patch 1: align with trunk what can/needs to be (cosmetics);
>> - patch 2: optimizations and correctness which don't seem to have
>> bitten us so far (not a proven fix someow);
>> - patch 3: a wakeup fix (corner case) that applies almost cleanly
>> thanks to 1/ and 2/.
>>
>> Would this work or should I go with 3/ directly and resolve backport
>> conflicts there?
>> Or maybe go with 3/ then 2/ then 1/, for the same result but at least
>> distros would care of the first step only (for this time...)?
> 
> Looks reasonable to me, better to rip the band-aid off then pay the
> price/risk every time something needs to be backported.
> 

+1

Regards

Rüdiger

Re: Align worker's worker's fdqueue with event's?

[Jim Jagielski <ji...@jaguNET.com>]

> On Jan 12, 2018, at 7:32 AM, Eric Covener <co...@gmail.com> wrote:
> 
> On Fri, Jan 12, 2018 at 6:51 AM, Yann Ylavic <ylavic.dev@gmail.com <ma...@gmail.com>> wrote:
>> A bit orthogonal, I'd also like to sync 2.4.x "event" with trunk's
>> w.r.t. cosmetic changes before (and to help) further backport
>> proposals.
>> 
>> That's possibly something that'll help *us* for later backports, but
>> not necessarily distros with (security-)fixes only policy.
>> Is that something we should more care about? I suppose distro
>> maintainers do care...
>> 
>> For instance, the three attached patches are how I would stage latest
>> "event" changes in 2.4.x:
>> - patch 1: align with trunk what can/needs to be (cosmetics);
>> - patch 2: optimizations and correctness which don't seem to have
>> bitten us so far (not a proven fix someow);
>> - patch 3: a wakeup fix (corner case) that applies almost cleanly
>> thanks to 1/ and 2/.
>> 
>> Would this work or should I go with 3/ directly and resolve backport
>> conflicts there?
>> Or maybe go with 3/ then 2/ then 1/, for the same result but at least
>> distros would care of the first step only (for this time...)?
> 
> Looks reasonable to me, better to rip the band-aid off then pay the
> price/risk every time something needs to be backported.

+1

BTW, there is no need for some of this stuff to be part of the public
API.

Re: Align worker's worker's fdqueue with event's?

[Eric Covener <co...@gmail.com>]

On Fri, Jan 12, 2018 at 6:51 AM, Yann Ylavic <yl...@gmail.com> wrote:
> A bit orthogonal, I'd also like to sync 2.4.x "event" with trunk's
> w.r.t. cosmetic changes before (and to help) further backport
> proposals.
>
> That's possibly something that'll help *us* for later backports, but
> not necessarily distros with (security-)fixes only policy.
> Is that something we should more care about? I suppose distro
> maintainers do care...
>
> For instance, the three attached patches are how I would stage latest
> "event" changes in 2.4.x:
> - patch 1: align with trunk what can/needs to be (cosmetics);
> - patch 2: optimizations and correctness which don't seem to have
> bitten us so far (not a proven fix someow);
> - patch 3: a wakeup fix (corner case) that applies almost cleanly
> thanks to 1/ and 2/.
>
> Would this work or should I go with 3/ directly and resolve backport
> conflicts there?
> Or maybe go with 3/ then 2/ then 1/, for the same result but at least
> distros would care of the first step only (for this time...)?

Looks reasonable to me, better to rip the band-aid off then pay the
price/risk every time something needs to be backported.

Re: Align worker's worker's fdqueue with event's?

[Yann Ylavic <yl...@gmail.com>]

A bit orthogonal, I'd also like to sync 2.4.x "event" with trunk's
w.r.t. cosmetic changes before (and to help) further backport
proposals.

That's possibly something that'll help *us* for later backports, but
not necessarily distros with (security-)fixes only policy.
Is that something we should more care about? I suppose distro
maintainers do care...

For instance, the three attached patches are how I would stage latest
"event" changes in 2.4.x:
- patch 1: align with trunk what can/needs to be (cosmetics);
- patch 2: optimizations and correctness which don't seem to have
bitten us so far (not a proven fix someow);
- patch 3: a wakeup fix (corner case) that applies almost cleanly
thanks to 1/ and 2/.

Would this work or should I go with 3/ directly and resolve backport
conflicts there?
Or maybe go with 3/ then 2/ then 1/, for the same result but at least
distros would care of the first step only (for this time...)?