Re: [reSIProcate] New #define landmine; RESIP_USE_STL_STREAMS

["Justin Matthews" <jmatthewsr@xxxxxxxxx>]

Right, for libraries built one way and used another.  Ok.  

FYI, the efficiency gains on *unix were not fully realized because the
implementation uses sprintf.  On windows there are api's for conversion that
are faster than sprintf.  It would be possible to use custom conversions
(there are a few in resip::Data).  Maybe it wouldn't get to the same
performance as windows, but it would improve by replacing sprintf.  

I wouldn't say it's throwing caution to the wind by deprecating STL streams
for encoding. I admit that I do not use resip on *unix systems and there are
some areas of functionality that I haven't used and so these areas need
testing. From my own experience on windows the implementation is solid and
has been used in mission-critical scenarios for years.  At some point it
will be tested on *unix.  IMHO it's a step forward in improving the speed
and efficiency of resip.

If the #define is in a header that's included in implementation files only,
will this work in all cases where inline functions use encoding functions
(or template function, or anything in a header file), or would it be
necessary to move them to implementation files?

Thanks,

-justin      

-----Original Message-----
From: Byron Campen [mailto:bcampen@xxxxxxxxxxxx] 
Sent: Wednesday, October 08, 2008 9:38 PM
To: Justin Matthews
Cc: 'resiprocate-devel'
Subject: Re: [reSIProcate] New #define landmine; RESIP_USE_STL_STREAMS

        API/ABI mismatch will occur if you use installed headers and fail to

define RESIP_USE_STL_STREAMS in the same way it was when the libraries  
were built. A header stating that DataStream has a base-class of  
std::iostream and a library where DataStream has a base of  
ResipFastOStream (or vice-versa) are not going to be compatible.

        The efficiency gains on Windows might be enough to get many to throw

caution to the wind, but the gains on linux are not as large. Outright  
deprecating the std::ostream-based code is probably not going to sit  
well with many who use resip on linux, unix, or OS X yet.

        DataStream would use std::iostream always. FastDataStream would use

ResipFastOStream always. The various encode() and operator<< functions  
could either have two versions (one for std::ostream and one for  
ResipFastOStream), or could be made template functions (ie; work with  
everything that has the right ostream-like functions implemented), or  
could be based on EncodeStream like now, except we define EncodeStream  
in a header file that is not installed, and only include the header in  
implementation files.

Best regards,
Byron Campen

> Hi Byron,
>
> What mismatches have you seen? On what platforms?  Note that by  
> default
> resip should compile exactly the same as it did before the fast  
> streams
> addition.  Let me know if that is not the case.
>
> Personally, I would like to see the STL interface removed.  Trying to
> support 2 interfaces seems, well, redundant.  The encoding mechanism  
> doesn't
> require the generic nature of the STL streams.  Yes, the STL streams
> interface is well defined and has been used quite a bit for a lot of  
> general
> solutions, but the performance gain with the alternative interface is
> significant.
>
> Of course removing an interface that has been used for so long is a  
> major
> change.  The general consensus may be that people feel more  
> comfortable
> having the option of either. If these new classes were created, how  
> would
> for example, resip::DataStream be used to determine which interface  
> to use?
>
>
> Thanks,
>
> -justin
>
>
> -----Original Message-----
> From: resiprocate-devel-bounces@xxxxxxxxxxxxxxx
> [mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxx] On Behalf Of  
> Byron Campen
> Sent: Wednesday, October 08, 2008 7:19 PM
> To: resiprocate-devel
> Subject: [reSIProcate] New #define landmine; RESIP_USE_STL_STREAMS
>
>       I'm seeing a new #define being used to substantively change header
> files; RESIP_USE_STL_STREAMS. This will cause nasty API/ABI mismatch  
> issues
> when working with installed headers. We really need to restrict this  
> kind of
> thing to implementation files wherever possible. So, I think we need  
> to
> define new classes, FastDataStream, FastoDataStream,  
> FastiDataStream, and
> FastDataBuffer to work alongside their non-fast counterparts, and  
> either
> write extra versions of the various and sundry operator<<, or perhaps
> convert them into template functions.
>
> Best regards,
> Byron Campen
>
>

Attachment: smime.p7s
Description: S/MIME cryptographic signature