Re: [PATCH] Delete server-side deleteThis()

On 10/13/2011 05:39 PM, Alex Rousskov wrote:
> On 10/13/2011 07:50 AM, Tsantilas Christos wrote:
>> On 10/13/2011 02:55 PM, Amos Jeffries wrote:
>>> On 13/10/11 22:02, Tsantilas Christos wrote:
>>>> This patch finishes the conversion of ServerStateData into AsyncJob by
>>>> properly implementing the doneAll() method and by removing calls to
>>>> deleteThis() or replacing with mustStop() calls as appropriate.
>
>>> In the new doneAll() you seem to have dropped:
>>> -#if USE_ADAPTATION
>>> - Adaptation::Initiator::doneAll()&&
>>> - BodyProducer::doneAll()&&
>>> -#endif
>>> - BodyConsumer::doneAll()
>>>
>>> ... IIRC that goes against the AsynJob API requirements. If it is
>>> supposed to be dropped please add a comment about why.
>
>> None of the above "doneAll()" methods is really implemented. The default
>> AsynJob::doneAll() used for all of them. The default doneAll() returns
>> always true.
>
> Christos,
>
> While you are correct, I do agree with Amos. We should add those
I have not any problem to add them, or add the required comments. I
agree too.
In my last mail I am just saying why they are removed...

> calls back even though they all _currently_ return true. Checking with
> parents is a part of the doneAll() API. You can place those always-true
> calls last to minimize their tiny performance impact, if any.

We can put them inside an #if 0 ... #endif in the worst case...
Give me some time, I will post the new patch...

>
> Sorry I missed that myself.
>
> Thank you,
>
> Alex.
>
Received on Thu Oct 13 2011 - 16:17:11 MDT