[ previous ] [ next ] [ threads ]
 To :  yate@v...
 From :  Allan Sandfeld Jensen <linux@c...>
 Subject :  Re: [yate] Patch storm
 Date :  Mon, 14 Jun 2010 18:21:59 +0200
On Monday 14 June 2010, Paul Chitescu wrote:
> 
> In this case a += a turns into a.operator+=(a.operator const char*). Other
> similar operations like a += a.c_str()+3 would have the same problem.
> 
> While such operation is not currently used in Yate code there are cases
> where the assignment operator is used to cut off from the start of the
> string like a = a.c_str()+3 and one may extend that to the appending
> operator unaware of its limitations.
> 
> Did you check how much improvement would bring this patch after fixing the
> tight loops in the (un)escape methods which I intend to apply?
> 
This was my first string optimization before improving escape/unescape. It 
made a big impact before. It still rates high in the profiling, but mainly due 
to output in the sniffer. The sniffer concatinates a lot of strings. With 
optimized escape/unescape and without the sniffer, this change is probably not 
that important. But before I made this optimization string handling more than 
50% of all YATE cpu time. 
I also like this patch because it mainly optimizes the worst case sceneario of 
adding one char at a time. All malloc implementations allocate memory in 8 or 
16 byte aligned pieces. This makes a minor realloc practically free the 
majority of times, a quick simple 8-16 times speed-up. Of course we should 
avoid string appends of that kind, but it is just so convinient in a lot of 
places, and a prealloced string construction is much easier to make mistakes 
in.

> 
> BTW, I noticed the change in String::safe():
> 
> return this ? ...
> 
> If "this" is unexpectedly NULL everything is already too screwed for the
> code to continue to run, better crash immediately.
> 
> If the intention is to enable coding s->c_str() without checking first if s
> is NULL - please don't do such a thing, use TelEngine::c_safe(s) instead.
> 
Hmm, yeah, it was just an extra safety because I made a few mistakes in 
debugging output of message parameters at one time. It is harmless though. It 
catches the case when you are working on variables of the type 'String*' like 
the result of calling getParam(). The result is then safe to call ::safe on, 
even if it is actually a null pointer. If the rest of the code handles the 
pointer correctly, then it doesn't matter that the debugging output is safely 
treating a null pointer like a string. I admit it is ugly though, and I am not 
relying on it anywhere. 

Regards
Allan