On 05/21/2009 02:01 PM, Andrei Pelinescu-Onciul wrote:
On May 21, 2009 at 13:22, Daniel-Constantin Mierla
<miconda(a)gmail.com> wrote:
On 05/21/2009 12:10 PM, Jan Janak wrote:
On 20-05 19:44, Daniel-Constantin Mierla wrote:
On 05/20/2009 07:31 PM, Andrei Pelinescu-Onciul
wrote:
> On May 20, 2009 at 19:24, Daniel-Constantin Mierla <miconda(a)gmail.com> wrote:
>
>
>
>> Hello,
>>
>> what would be the drawback of having sip_msg being all the time in
>> shared memory? Would pkg vs shm operations have relevant impact?
>>
>>
>>
> Yes.
>
>
>
>
>> From personal observations, most of the requests (over 95%) end in TM
>> module (to absorb retransmission or to forward) where the sip_msg s
>> moved to shm. It would make things simpler for tm callbacks and related
>> routes (no need to move back/forward from/to pkg/shm). Parsing will
>> happen always once, as now cloning to shm in tm discards some parsed
>> headers, which may be needed in failure route or callbacks.
>>
>>
>>
> Havin a non-shm copy helps with locking and with moving cache lines
> between cpus.
>
>
>
as lot of processing is done with the message in shm anyhow, I tried to
figure out the impact of the rest of processing. So you say that is
better to copy to pkg and re-parse, than working with shm structures.
The access is not sync'ed, as there will be same usage - in one process
- just the alloc/free will be different.
I think it would make sense to make the memory allocator used in parsers
somehow configurable, so that we can select whether the parsed structure is
stored in shared or in private memory.
Not only this could save us some memory copying in modules like tm, but we
could also experiment with using the shared memory directly in some parts of
the code.
Yes, being configurable will make happy everyone.
While I agree with making the parse function use a configurable memory
allocator, I don't agree with parsing in shm from the beginning.
That would be useful in an extremely limited number of scenarios (e.g.
config containing only t_relay()).
In most configs there are different decisions or routing "branches" that
don't go through tm (e.g. if (...) drop;).
Yes, there are lot of branches in the config logic that could end up in
drop, but the chances to hit them are low comparing with good traffic
that ends to a call/registration/presence update/etc...
Such cases for example are max fwd hit, malformed messages, etc ... and
unless there is a DoS, most of traffic ends in TM.
Forcing shm allocation for
every packet would lead to a significant performance drop, especially on
machines with lots of cpu/cores (it scales terribly).
Apart from that there are the other problems I've mentioned.
The pkg_malloc() part takes an insignificant amount of time compared to
shm. Even with the memcpy() when cloning to shm we loose very little
performance when compared to going shm from the beginning for all the
packets (even for ones that won't be relayed/handled through tm).
We could have this discussion again once we have new shm memory
allocators (we have already some semi-experimental stuff, but it still
not on-par with fmalloc on single or dual cpus, much more work has to go
into them).
Until then and unless someone is able to show me some hard evidence (tests),
I won't agree with going shm from the beginning.
I don't have any figures and I am not willing to get to shm if the
performance impact is significant negative. I just asked to see opinions
and now looking forward to your new shm manager, whenever is going to be
ready...
Since we are here, couple more thoughts about
discussions we had in the
past on various mailing lists:
- when there are changes in the parsed headers structure, there are
other parts of code forgotten to be updated, usually TM. I was thinking
it might worth to change the parsed field from void* to pointer to a
structure like:
{
void *data;
void free_function(void *data);
void *clone_function(void *date, malloc_function, free_function);
}
Drawback is two more pointers, but if we remove most of "very unused"
direct header hooks, then we have same spare space...
It wastes too much space for no good reason. The forget to clone new
headers in tm problem is already handled by the compiler: a warning will
be generated if someone forgets to add a header to sip_msg_cloner.
It also happens that the header is added (with or without need to clone
the parsed field), and latter something changes. It is not only tm,
there are other modules that need header cloning (e.g., dialog,
presence, ...). Then it is the problem with HDR_OTHER_T, keep adding
types and flags won't scale with current sip paranoia of extensibility.
We can reduce with one pointer by having the pointer to the data and a
pointer to a structure holding various functions:
structure like:
{
void *data;
pointer-to {
void free_function(void *data);
void *clone_function(void *date, malloc_function, free_function);
}
}
... and remove from header hooks :-) ...
Instead of the 2 function callbacks, we could add a
"flags" member in which
we could mark if the header is alloc'ed in shm, pkg, should be freed a.s.o.
The issue is that the parsed structure might not be one chunk, but a
complex structure.
Daniel
Advantage will
be automatic and easy cloning/freeing.
Andrei
--
Daniel-Constantin Mierla
http://www.asipto.com/