Re: CORBA::string_dup

Serguei Kolos <[email protected]> Mon, 06 Oct 2003 18:32:08 +0200

Newsgroups	gmane.comp.corba.omniorb.devel
Message-ID	<[email protected]>


Duncan Grisby wrote:

>On Friday 26 September, Serguei Kolos wrote:
>
>  
>
>>I have noticed that the CORBA::string_dup function uses strcpy to copy 
>>strings.
>>Unfortunately on many systems strcpy is much slower then for example 
>>memcpy.
>>For example on Linux (RedHat 7.3) the strcpy is 5 (sic!) times slower. 
>>    
>>
>
>Really?  Wow!  I wonder what it's doing.
>
>  
>
>>What do you think about the idea of using the memcpy in the
>>CORBA::string_dup instead of the strcpy? Are there any drawbacks
>>with the memcpy?
>>    
>>
>
>I suspect that on other platforms memcpy might be slower than strcpy.
>
I have tested on Solaris 5.7 with SunPro 5.2. The memcpy is still faster 
then strcpy (bu not so dramatic
as on Linux).

>
>Something to try is to replace the call with the obvious for loop.
>Between omniORB 3 and 4 I did some profiling and found that strcmp was
>stupidly slow, and by far the fastest thing was an inline function
>with a for loop. It will probably be the same with strcpy.
>  
>
I have tested this also and found that the for loop solution is not 
always faster in
case of strcpy and strlen (on Solaris for example). It depends on 
OS/compiler.
I attached simple program that makes timing for various strdup 
implementations.
You can see that the fastest one is always system strdup, but as far as 
I know
the strdup is not available on all the platforms. But note also a 
difference between strcpy
and memcpy.

In my PIII 800 (RedHat 7.3 with g++ 2.95.2) box the output is:
(built with g++ -O3)

system strdup 2.41994
my_strdup (for loop for copy and length) 6.97561
my_strdup1 (strlen + strcpy) 10.8472
my_strdup2 (for loop for length + memcpy) 6.3582
my_strdup3 (strlen + memcpy)7.16166
strcpy only 3.12776
memcpy only 0.565542

Note the last two lines - times differ in 6 times.

On Solaris with SunPro 5.2 situation with strcpy is not so dramatic:
(built with CC -xO4)

system strdup 7.86524
my_strdup (for loop for copy and length) 15.1074
my_strdup1 (strlen + strcpy) 8.08667
my_strdup2 (for loop for length + memcpy) 12.9627
my_strdup3 (strlen + memcpy)7.29567
strcpy only 3.63349
memcpy only 2.71386

But again the fastest one is "strlen + memcpy".

And finally here are the results from dual PIII 1Ghz (RedHat 9 with g++ 
3.3.1)
(built with g++ -O3)
The memcpy is 6 !!! times faster.
The fastest one is again system strdup.

system strdup 1.70942
my_strdup (for loop for copy and length) 5.44478
my_strdup1 (strlen + strcpy) 6.53631
my_strdup2 (for loop for length + memcpy) 4.92023
my_strdup3 (strlen + memcpy)4.81977
strcpy only 2.08511
memcpy only 0.36471

I don't know what the strcpy is doing here.

Finally I believe it would be wise to change strcpy to something else in 
string_dup.

Cheers,
Sergei

_______________________________________________
omniORB-dev mailing list
[email protected]
http://www.omniorb-support.com/mailman/listinfo/omniorb-dev

test_copy.cc (text/plain, 3 KB)

#include <iostream.h>
#include <stdlib.h>
#include <string.h>
#include <sys/time.h>

using namespace std;

#define SIZE 1024

char * my_strdup( const char * s )
{
	register const char *p = s;
	for ( ;*p++; );
	
	char * d = new char[p-s+1];
	
	p = s;
	register char * r = d;
	for ( ;*p++; ) 
		*r++ = *p++;
	*r = 0;
	return d;
}

char * my_strdup1( const char * s )
{
	int i = strlen( s );
	
	char * d = new char[i+1];
	
	strcpy( d, s );
	
	return d;
}

char * my_strdup2( const char * s )
{
	const char *p = s;
	for ( ;*p++; );
	
	char * d = new char[p-s+1];
	
	memcpy( d, s, p-s+1 );
	return d;
}

char * my_strdup3( const char * s )
{
	int i = strlen( s );
	
	char * d = new char[i+1];
	
	memcpy( d, s, i+1 );
	return d;
}

void my_strdup4( char * d, const char * s )
{
	strcpy( d, s );
}

void my_strdup5( char * d, const char * s )
{
	memcpy( d, s, SIZE );
}


int main()
{
	struct timeval start, stop;
	int i;
	float time;
	
	char buff[SIZE];
	memset( buff, '1', SIZE-1 );
	buff[SIZE-1] = 0;
		
	gettimeofday( &start, 0 );
	for ( i = 0; i < 1000000; i++ )
	{
		char * d = strdup( buff );
		free( d );
		buff[24] = '2';
	}
	gettimeofday( &stop, 0 );
	
	time = ( stop.tv_sec - start.tv_sec ) * 1000000 + ( stop.tv_usec - start.tv_usec );
	cout << "system strdup " << time/1000000. << endl;
	
	gettimeofday( &start, 0 );
	for ( i = 0; i < 1000000; i++ )
	{
		char * d = my_strdup( buff );
		free( d );
	}
	gettimeofday( &stop, 0 );
	
	time = ( stop.tv_sec - start.tv_sec ) * 1000000 + ( stop.tv_usec - start.tv_usec );
	cout << "my_strdup (for loop for copy and length) " << time/1000000. << endl;
	
	gettimeofday( &start, 0 );
	for ( i = 0; i < 1000000; i++ )
	{
		char * d = my_strdup1( buff );
		delete[] d;
	}
	gettimeofday( &stop, 0 );
	
	time = ( stop.tv_sec - start.tv_sec ) * 1000000 + ( stop.tv_usec - start.tv_usec );
	cout << "my_strdup1 (strlen + strcpy) " << time/1000000. << endl;
	
	gettimeofday( &start, 0 );
	for ( i = 0; i < 1000000; i++ )
	{
		char * d = my_strdup2( buff );
		delete[] d;
	}
	gettimeofday( &stop, 0 );
	
	time = ( stop.tv_sec - start.tv_sec ) * 1000000 + ( stop.tv_usec - start.tv_usec );
	cout << "my_strdup2 (for loop for length + memcpy) " << time/1000000. << endl;
	
	gettimeofday( &start, 0 );
	for ( i = 0; i < 1000000; i++ )
	{
		char * d = my_strdup3( buff );
		delete[] d;
	}
	gettimeofday( &stop, 0 );
	
	time = ( stop.tv_sec - start.tv_sec ) * 1000000 + ( stop.tv_usec - start.tv_usec );
	cout << "my_strdup3 (strlen + memcpy) " << time/1000000. << endl;
	
	gettimeofday( &start, 0 );
	char * d = new char[strlen(buff)+1];
	for ( i = 0; i < 1000000; i++ )
	{
		my_strdup4( d, buff );
	}
	gettimeofday( &stop, 0 );
	
	time = ( stop.tv_sec - start.tv_sec ) * 1000000 + ( stop.tv_usec - start.tv_usec );
	cout << "strcpy only " << time/1000000. << endl;
	
	gettimeofday( &start, 0 );
	for ( i = 0; i < 1000000; i++ )
	{
		my_strdup5( d, buff );
	}
	gettimeofday( &stop, 0 );
	
	time = ( stop.tv_sec - start.tv_sec ) * 1000000 + ( stop.tv_usec - start.tv_usec );
	cout << "memcpy only " << time/1000000. << endl;
	
}