You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by ge...@everythingsucks.co.uk on 2006/01/05 01:42:35 UTC
Re: UPDATE: [Patch] poll/unix/poll.c:apr_pollset_poll()
optimisation
Quoting Garrett Rooney <ro...@electricjellyfish.net>:
> Um, I don't think that's going to work, rv is the number of
> descriptors that hit, there's nothing that says that if N hit it'll be
> the first N...
That would be true were I comparing I<RV but I'm comapring J<RV :D
> As for the other version of this patch, I'm kind of curious if it's an
> actual improvement. Sure, you can bail early, but at the cost of
> intruducing an extra if into each of the iterations before you bail.
> Is that actually better in the average case?
Yeah I kinda thought the same thing, was bugging me which is why I
revisited it.
G
> On 1/4/06, gerry@everythingsucks.co.uk <ge...@everythingsucks.co.uk> wrote:
>> Greetings,
>>
>> There's a sneakier (and pretty obvious, once you look at it) way of
>> improving this loop which I prefer to my original mail by replacing the
>> for-loop condition.
>>
>> Should be safe: I like poll()... he has an honest face... I don't think
>> he will lie to us about the number of signalled descriptors.
>>
>>
>> --- poll.c 2006/01/02 18:30:27 1.1
>> +++ poll.c 2006/01/05 00:11:36
>> @@ -254,13 +254,13 @@
>> return apr_get_netos_error();
>> }
>> if (rv == 0) {
>> return APR_TIMEUP;
>> }
>> j = 0;
>> - for (i = 0; i < pollset->nelts; i++) {
>> + for (i = 0; j < rv; i++) {
>> if (pollset->pollset[i].revents != 0) {
>> pollset->result_set[j] = pollset->query_set[i];
>> pollset->result_set[j].rtnevents =
>> get_revent(pollset->pollset[i].revents);
>> j++;
>> }
>>
>
> Um, I don't think that's going to work, rv is the number of
> descriptors that hit, there's nothing that says that if N hit it'll be
> the first N...
>
> As for the other version of this patch, I'm kind of curious if it's an
> actual improvement. Sure, you can bail early, but at the cost of
> intruducing an extra if into each of the iterations before you bail.
> Is that actually better in the average case?
>
> -garrett
>
Re: UPDATE: [Patch] poll/unix/poll.c:apr_pollset_poll()
optimisation
Posted by ge...@everythingsucks.co.uk.
Quoting "Roy T. Fielding" <fi...@gbiv.com>:
> On Jan 4, 2006, at 4:42 PM, gerry@everythingsucks.co.uk wrote:
>
>> Quoting Garrett Rooney <ro...@electricjellyfish.net>:
>>
>>> Um, I don't think that's going to work, rv is the number of
>>> descriptors that hit, there's nothing that says that if N hit it'll be
>>> the first N...
>>
>> That would be true were I comparing I<RV but I'm comapring J<RV :D
>
>>>> - for (i = 0; i < pollset->nelts; i++) {
>>>> + for (i = 0; j < rv; i++) {
>
> I have a vague recollection that there was some reason we couldn't
> trust the return value being equal to the number of ready descriptors
> on some platform, but I have no time to search the archives for why.
> I may be remembering something related to ... er .. that other call
> like poll... hmmm, oh, select. Or maybe I just need more caffeine.
>
> ....Roy
>
You are correct, select() returns the total number of descriptors in
the FDSET rather than those which have had events.
However the patch applies specifically to the poll() implementation for
pollset, select() remains unchanged for that very reason.
Re: UPDATE: [Patch] poll/unix/poll.c:apr_pollset_poll() optimisation
Posted by "Roy T. Fielding" <fi...@gbiv.com>.
On Jan 4, 2006, at 4:42 PM, gerry@everythingsucks.co.uk wrote:
> Quoting Garrett Rooney <ro...@electricjellyfish.net>:
>
>> Um, I don't think that's going to work, rv is the number of
>> descriptors that hit, there's nothing that says that if N hit
>> it'll be
>> the first N...
>
> That would be true were I comparing I<RV but I'm comapring J<RV :D
>>> - for (i = 0; i < pollset->nelts; i++) {
>>> + for (i = 0; j < rv; i++) {
I have a vague recollection that there was some reason we couldn't
trust the return value being equal to the number of ready descriptors
on some platform, but I have no time to search the archives for why.
I may be remembering something related to ... er .. that other call
like poll... hmmm, oh, select. Or maybe I just need more caffeine.
....Roy