Bill Stoddard wrote:
>
>>On Sat, Apr 27, 2002 at 07:30:51PM -0700, Justin Erenkrantz wrote:
>>
>>>> + qi = apr_palloc(pool, sizeof(*qi));
>>>> + memset(qi, 0, sizeof(*qi));
>>>>
>>>As we said, if you are concerned about the performance aspect
>>>of apr_pcalloc, then we should fix apr_pcalloc NOT attempt to
>>>work around its inefficiencies by pointedly not using it.
>>>
>>>If/when Cliff (or someone else?) commits the change to apr_pcalloc,
>>>this chunk would be magically changed along with everything else if
>>>you simply called apr_pcalloc in the first place.
>>>
>>We don't have a consensus on this, and I'm ambivalent about making the
>>p_calloc macro. If we do come up with a consensus than it can change.
>>Until then this is more correct than using p_calloc.
>>
>
>I have no problem with using apr_palloc()/memset() in place of apr_pcalloc().
>
I'm +1 on turning pcalloc into a macro,
-0 for using pcalloc in ap_queue_info_init(),
-0.5 (non-veto) for using palloc/memset in ap_queue_info_init()
Rationale:
* Changing pcalloc to a macro will give us a free optimization
throughout the entire code base.
* In ap_queue_info_init(), we create a struct with 7 fields and
immediately set 4 of them to nonzero values. Using pcalloc, or
any other form of block zero-fill, isn't a big win.
* The whole point of the palloc/memset idiom is that
it's a more awkward syntax than pcalloc, but justified
for the sake of performance optimization. But there's
no point in optimizing ap_queue_info_init(). The function
gets called once per child proc, at startup, and no matter
what technique you use to zero-fill the newly created struct,
it's going to account for far too little of the total run
time to matter at all. There's plenty of code within
fdqueue.c that could benefit from performance optimization,
but none of it is in this function.
>>>> + rv = apr_thread_mutex_unlock(queue_info->idlers_mutex);
>>>> + if (rv != APR_SUCCESS) {
>>>> + return rv;
>>>> + }
>>>> + return APR_SUCCESS;
>>>> +}
>>>>
>>>As I said before, simply "return rv;" works here.
>>>
>>Yeah, but this is much more readable.
>>
>
>I disagree. I would rather see "return rv".
>
I'd rather see "return rv" or "return apr_thread_mutex_unlock".
Both are more readable (and faster) than the current code with
the if-statement.
--Brian
