views:

4223

answers:

9

Hi, I came across these two methods to concatenate strings:

Common part:

char* first= "First";
char* second = "Second";
char* both = malloc(strlen(first) + strlen(second) + 2);

Method 1:

strcpy(both, first);
strcat(both, " ");
strcat(both, second);

Method 2:

sprintf("%s %s", first, second);

In both cases the content of both would be "First Second".

I would like to know which one is more efficient (I have to perform several concatenation operations), or if you know a better way to do it.

Thanks for your time.

EDIT: In the first case the space could have been within one of the strings.

A: 

Neither is terribly efficient since both methods have to calculate the string length or scan it each time. Instead, since you calculate the strlen()s of the individual strings anyway, put them in variables and then just strncpy() twice.

dajobe
why should he use `strncpy()` if he knows the strings' sizes? `memcpy()` is your friend!
Christoph
+16  A: 

Don't worry about efficiency: make your code readable and maintainable. I doubt the difference between these methods is going to matter in your program.

Ned Batchelder
I am with Ned. It seems like you are performing premature optimisation. Like girls, it is also the root of all evil (it has multiple roots). Get your program running, then profile it, then optimise. Until then you are just waiting time IMHO.
freespace
Fair enough, thanks for your comments. I think that doing some things in a better manner from the beginning can save time later on, and this one may be one.Thanks once more.
Xandy
@Xandy: you're right that doing things better to start with is good, but in practice faster code usually isn't "better" to debug and maintain. I always end up spending way more time debugging and refactoring my code, and adding more features, than I spend speeding it up. So personally I'd either use s(n)printf (or even a string library that handles allocation) to optimise simplicity of the source code, or if optimising for speed I'd do an absolute bare minimum of memory access (and hence not use strcat when I already have the lengths).
Steve Jessop
@Ned: That doesn't answer the question! He asked which way is more efficient, not if he should worry about efficiency or not.
Wadih M.
@Wadih: it's an answer that points out an unexamined assumption and thus makes the OP's question irrelevant. It's about the higher goal of efficiency in development. Both make it an appropriate answer. http://catb.org/~esr/faqs/smart-questions.html#goal
outis
@outis: A concatenation extensive tasks (string analysis, text manipulation, text generators) must take into account the performance of the different possible implementations, and this is a totally valid and smart question. Educating onself on the different approaches possible to resolve a problem before beginning implementation is the smartest thing to do. Asthetics and maintainability are only TWO factors to consider. Performance is another and there are plenty of cases where the latter is the dominant one.
Wadih M.
Bottom line - nobody needs the permission of StackOverflow to optimise their code, but when StackOverflowers say "optimising this will have no measurable effect on the performance of your program, and will make your code harder to maintain", they're right more often than not.
Steve Jessop
I agree, I tend to write readable code yet efficient, but this time efficiency when handling strings may be determinative. Using memcpy() (over strcat()/strcpy() and sprintf()) seems to be the fastest way across several implementations but it's not as easy to maintain as other methods may be. Anyway a profiler will give me more info when the time comes. Thank you for all your outstanding support and time, it's much appreciated.
Xandy
+2  A: 

They should be pretty much the same. The difference isn't going to matter. I would go with sprintf since it requires less code.

Jay Conrod
+11  A: 

For readability, I'd go with

char * s = malloc(snprintf(NULL, 0, "%s %s", first, second) + 1);
sprintf(s, "%s %s", first, second);

If your platform supports GNU extensions, you could also use asprintf():

char * s = NULL;
asprintf(&s, "%s %s", first, second);

If you're stuck with the MS C Runtime, you have to use _scprintf() to determine the length of the resulting string:

char * s = malloc(_scprintf("%s %s", first, second) + 1);
sprintf(s, "%s %s", first, second);

The following will most likely be the fastest solution:

size_t len1 = strlen(first);
size_t len2 = strlen(second);

char * s = malloc(len1 + len2 + 2);
memcpy(s, first, len1);
s[len1] = ' ';
memcpy(s + len1 + 1, second, len2 + 1); // includes terminating null
Christoph
I'd just like to put in a vote of disagreement for your first solution being readable. It's more compact, but is it more readable? I don't think so. I didn't downvote, though.
Imagist
It would perhaps be worth mentioning `asprintf()` which does the memory allocation for you: `char *s; int len = asprintf(` without any fuss or muss.
Jonathan Leffler
@Jonathan: `asprintf()` isn't part of the C stdlib and the MS-compiler dosn't support it
Christoph
@PS: on second thought, you might have to use `_scprintf()` to determine the size of the buffer anyway as I'm not sure if `_snprintf()` from the MS CRT behaves as required by the standard when the size argument is `0`
Christoph
@Christoph: yes, I know asprintf() is not standard; that's why I suggested mention it rather than proposing it as 'the answer'. Perhaps I should have put in the relevant caveats in my original comment, though. (Man page at: http://linux.die.net/man/3/asprintf, amongst other places.)
Jonathan Leffler
@Jonathan: thx, I added examples for GNU and MS
Christoph
+6  A: 
size_t lf = strlen(first);
size_t ls = strlen(second);

char *both = (char*) malloc((lf + ls + 2) * sizeof(char));

strcpy(both, first);

both[lf] = ' ';
strcpy(&both[lf+1], second);
Michalis Giannakidis
That strcat should be a second strcpy - this is undefined behavior as written.
Steve Jessop
+1 - you spotted the buffer overflow!
Jonathan Leffler
In fact, one could use memcpy, since the length are already calculated :)
Filip Navara
But, as @onebyone points out, the strcat() is not OK this time, because the strcat() starts tracking after the space, and you don't know what characters are in the string at that point.
Jonathan Leffler
The strcat is definitely a mistake. I will edit and fix it
Michalis Giannakidis
@Filip: actually, it's plausible that strcpy could be faster than memcpy. To use memcpy, you need to keep ls hanging around, which means using more registers, which could perhaps cost you a extra stack store before the call to malloc. The naive implementations of memcpy and strcpy have very similar inner loops, just mempcy decrements a length and checks 0, whereas strcpy compares the byte copied against 0. So it's all down to how ferociously optimised those two functions are in your implementation, which you'd have to investigate on a case-by-case basis :-)
Steve Jessop
@onebyone: optimized versions of `memcpy()` will copy multiple bytes per iteration step; `strcpy()` may also do this, but it still has to examine every single byte to check for the terminating 0; therefore I'd expect `memcpy()` to be faster
Christoph
Sure, I'd expect memcpy to be faster too for long strings. I just find it entertaining to speculate what might cause the counter-intuitive thing to happen. For instance, on some architectures (including MMX, I think) there are packed comparison ops, so strcpy could check all the bytes in a copied block in one instruction...
Steve Jessop
A: 

I don't know that in case two there's any real concatenation done. Printing them back to back doesn't constitute concatenation.

Tell me though, which would be faster:

1) a) copy string A to new buffer b) copy string B to buffer c) copy buffer to output buffer

or

1)copy string A to output buffer b) copy string b to output buffer

San Jacinto
+2  A: 

The difference is unlikely to matter:

  • If your strings are small, the malloc will drown out the string concatenations.
  • If your strings are large, the time spent copying the data will drown out the differences between strcat / sprintf.

As other posters have mentioned, this is a premature optimization. Concentrate on algorithm design, and only come back to this if profiling shows it to be a performance problem.

That said... I suspect method 1 will be faster. There is some---admittedly small---overhead to parse the sprintf format-string. And strcat is more likely "inline-able".

ijprest
The `strcat` version scans the full length of the `first` string four times, whereas the `sprintf` version only does so twice. So when the `first` string is very very long, the `strcat` version will eventually end up slower.
caf
+1  A: 

sprintf() is designed to handle far more than just strings, strcat() is specialist. But I suspect that you are sweating the small stuff. C strings are fundamentally inefficient in ways that make the differences between these two proposed methods insignificant. Read "Back to Basics" by Joel Spolsky for the gory details.

This is an instance where C++ generally performs better than C. For heavy weight string handling using std::string is likely to be more efficient and certainly safer.

[edit]

[2nd edit]Corrected code (too many iterations in C string implementation), timings, and conclusion change accordingly

I was surprised at Andrew Bainbridge's comment that std::string was slower, but he did not post complete code for this test case. I modified his (automating the timing) and added a std::string test. The test was on VC++ 2008 (native code) with default "Release" options (i.e. optimised), Athlon dual core, 2.6GHz. Results:

C string handling = 0.023000 seconds
sprintf           = 0.313000 seconds
std::string       = 0.500000 seconds

So here strcat() is faster by far (your milage may vary depending on compiler and options), despite the inherent inefficiency of the C string convention, and supports my original suggestion that sprintf() carries a lot of baggage not required for this purpose. It remains by far the least readable and safe however, so when performance is not critical, has little merit IMO.

I also tested a std::stringstream implementation, which was far slower again, but for complex string formatting still has merit.

Corrected code follows:

#include <ctime>
#include <cstdio>
#include <cstring>
#include <string>

void a(char *first, char *second, char *both)
{
    for (int i = 0; i != 1000000; i++)
    {
        strcpy(both, first);
        strcat(both, " ");
        strcat(both, second);
    }
}

void b(char *first, char *second, char *both)
{
    for (int i = 0; i != 1000000; i++)
        sprintf(both, "%s %s", first, second);
}

void c(char *first, char *second, char *both)
{
    std::string first_s(first) ;
    std::string second_s(second) ;
    std::string both_s(second) ;

    for (int i = 0; i != 1000000; i++)
        both_s = first_s + " " + second_s ;
}

int main(void)
{
    char* first= "First";
    char* second = "Second";
    char* both = (char*) malloc((strlen(first) + strlen(second) + 2) * sizeof(char));
    clock_t start ;

    start = clock() ;
    a(first, second, both);
    printf( "C string handling = %f seconds\n", (float)(clock() - start)/CLOCKS_PER_SEC) ;

    start = clock() ;
    b(first, second, both);
    printf( "sprintf           = %f seconds\n", (float)(clock() - start)/CLOCKS_PER_SEC) ;

    start = clock() ;
    c(first, second, both);
    printf( "std::string       = %f seconds\n", (float)(clock() - start)/CLOCKS_PER_SEC) ;

    return 0;
}
Clifford
A quick modification of my test (posted in a separate answer) revealed that converting method 1, with the malloc and free, into C++ using std::string was less than half the speed of the C version. The body of the loop was just "both = first + std::string(" ") + second;"However, the C++ is better in all kinds of other ways.
Andrew Bainbridge
Ah, reading the question again, I see how sprintf() would be faster that *two* strcat() calls, for the reasons mentioned in Joel's article. I am surprised that a std::string implementation was slower, but goes to show you have to measure if you need to know!
Clifford
Did you notice that method function a goes around its loop 48 times more than function b or function c? That was my dumb way of demonstrating the performance multiple. Posting the actual timings like you did is much more sensible.The timings I got on mingw gcc 4.4 (with the 48 times multiple removed) were:C string handling = 0.093000 secondssprintf = 0.266000 secondsstd::string = 0.766000 secondsAnd for Visual Studio 2005 (haven't got 2008 unfortunately):C string handling = 0.047000 secondssprintf = 0.343000 secondsstd::string = 0.485000 seconds
Andrew Bainbridge
Here are the timings (1000000 loop times for all) in a Core 2 Duo 2.0 GHz (all of them compiled without optimizations): Small strings: GCC 4.4: C string handling = 0.093 secs., sprintf = 0.328 secs, std::string = 1.560 secs. VC++ 2008: C string handling = 0.062 secs., sprintf = 0.296 secs., std::string = 1.498 secs. Intel C++ Compiler: C string handling = 0.109 secs. sprintf = 0.281 secs. std::string = 0.249 secs. Interesting results those of Intel's.
Xandy
Larger strings (120 and 140 characters each) and equal loops (1000000), all of them compiled from command line without optimizations (g++, cl and icl strings.cpp): GCC 4.4: C string handling = 0.250 secs., sprintf = 2.355 secs., std::string = 1.779 secs.; VC++ 2008: C string handling = 0.280 secs., sprintf = 2.216 secs., std::string = 4.836 secs.; Intel C++ Compiler: C string handling = 0.748 secs., sprintf = 2.465 secs., std::string = 3.214 secs. By the way, very interesting the article by Joel Spolsky.
Xandy
OK, lets get some perspective; The results are variable, but even the worst example had one million operations in less than 5 seconds. That's 5 microseconds per operation. Fast enough is good enough. There are situations where this may be critical, most often it is not. hence my suggestion about sweating teh small stuff. If you are moving a lot of strings, and need speed, measure it. Otherwise use what is simplest and safest.
Clifford
I might try an implementation using std::ostringstream for kicks and giggles. This is arguably more analogous to sprintf()'s behaviour.
Clifford
Sorry Andrew, I only just realised what you meant about the x48 loop! I have fixed the code and changed the timings and conclusion. Interesting how much worse GNU's std::string seems to be than VC++'s.
Clifford
In C the fastest way across several function implementations seems to be memcpy() (over strcpy()/strcat() and sprintf()). If using C++, std::stringstream can be another way to do it as Clifford points out (using redirectors << and then return str() for example).
Xandy
+4  A: 

Here's some madness for you, I actually went and measured it. Bloody hell, imagine that. I think I got some meaningful results.

I used a dual core P4, running Windows, using mingw gcc 4.4, building with "gcc foo.c -o foo.exe -std=c99 -Wall -O2".

I tested method 1 and method 2 from the original post. Initially kept the malloc outside the benchmark loop. Method 1 was 48 times faster than method 2. Bizarrely, removing -O2 from the build command made the resulting exe 30% faster (haven't investigated why yet).

Then I added a malloc and free inside the loop. That slowed down method 1 by a factor of 4.4. Method 2 slowed down by a factor of 1.1.

So, malloc + strlen + free DO NOT dominate the profile enough to make avoiding sprintf worth while.

Here's the code I used (apart from the loops were implemented with < instead of != but that broke the HTML rendering of this post):

void a(char *first, char *second, char *both)
{
    for (int i = 0; i != 1000000 * 48; i++)
    {
        strcpy(both, first);
        strcat(both, " ");
        strcat(both, second);
    }
}

void b(char *first, char *second, char *both)
{
    for (int i = 0; i != 1000000 * 1; i++)
        sprintf(both, "%s %s", first, second);
}

int main(void)
{
    char* first= "First";
    char* second = "Second";
    char* both = (char*) malloc((strlen(first) + strlen(second) + 2) * sizeof(char));

    // Takes 3.7 sec with optimisations, 2.7 sec WITHOUT optimisations!
    a(first, second, both);

    // Takes 3.7 sec with or without optimisations
    //b(first, second, both);

    return 0;
}
Andrew Bainbridge
Thanks for the benchmarking! It's really appreciated! Regarding the time spent with and without optimizations in the first case, -O2 may perform some optimizations which result in slower code in favour of smaller code (http://www.linuxjournal.com/article/7269). Thanks for your answer and time.
Xandy
Having just looked at the generated instructions, the -O2 code is bigger as well as slower! The problem looks to be that gcc is using the "repne scasb" instruction to find the length of the string. I suspect that that instruction is very slow on modern hardware. I'm going to find a gcc expert to ask about this.
Andrew Bainbridge
@Andrew Bainbridge, a little bit OT, but you can use < and > for < and >
quinmars
@Andrew Bainbridge: You can also indent by 4 spaces to format as code. Then you don't have to escape < and > and you also get syntax highlighting.
bk1e
Try using `-march=generic`. mingw defaults to i586 which is really really old, outdated and makes assumptions that will fit
LiraNuna
@LiraNuna: My gcc claims that -march=generic doesn't work but -mtune=generic does, and indeed fixes the problem (the repne is replaced with call _strlen). The resulting code is faster than the unoptimised! I had previously tried -march=pentium4, which didn't help.
Andrew Bainbridge
@LiraNuna: Thanks for the info, I didn't know that too.
Xandy