[SR-Dev] what if ...

[Daniel-Constantin Mierla]

On 05/21/2009 02:01 PM, Andrei Pelinescu-Onciul wrote:
> On May 21, 2009 at 13:22, Daniel-Constantin Mierla <miconda at 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 at 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/