|
3
|
|
|
Extraneous call to strlen() . Bare in mind that a C string, unlike its C++ std::string counterpart, does not carry extra information about its length, except for the null char that terminates it. So each call to strlen() is a full scan of the string to find out its length (count the chars til a null is found). You can easily speed up your code by calling strlen() once and saving the result:
char *mon_string_rev(char *string)
{
assert(string != NULL)
size_t input_len = strlen(string);
char *out = malloc(input_len + 1);
assert(out != NULL);
// use 'input_len' as many times as it is needed.
....
Also note that strlen() returns a size_t object (which is usually a typedef to unsigned long ). It is best to declare the variable that receives the return value as such.
Your function creates a copy of the input string with malloc() . This might be a requirement of the implementation, if so, disregard this item. Allocating dynamic memory is a costly operation, so high performance software should avoid it as much as possible. Plus, you place the burden of deallocating the buffer on the caller. Which is more likely to lead to memory leaks. A best option would be perhaps to receive a null terminated string and reverse it in-place. I once wrote an in-place string reversing function that used the XOR swap algorithm. If you want to compare it against yours here it is:
char * StringReverse(char * str)
{
assert(str != NULL);
if (!*str)
{
/* Empty string, do nothing */
return (str);
}
char * p1;
char * p2;
for (p1 = str, p2 = str + strlen(str) - 1; p2 > p1; ++p1, --p2)
{
/* XOR trick to swap integer values: */
*p1 ^= *p2;
*p2 ^= *p1;
*p1 ^= *p2;
}
return (str);
}
Unlike yours, this function will reverse the input string in-place, without malloc() . You can also easily write a wrapper function that mallocs a new string, passes it to StringReverse() and then returns it. The XOR swapping might or might not be faster, this will largely depend on the CPU architecture though.
Extraneous call to strlen() . Bare in mind that a C string, unlike its C++ std::string counterpart, does not carry extra information about its length, except for the null char that terminates it. So each call to strlen() is a full scan of the string to find out its length (count the chars til a null is found). You can easily speed up your code by calling strlen() once and saving the result:
char *mon_string_rev(char *string)
{
assert(string != NULL)
size_t input_len = strlen(string);
char *out = malloc(input_len + 1);
assert(out != NULL);
// use 'input_len' as many times as it is needed.
....
Also note that strlen() returns a size_t object (which is usually a typedef to unsigned long ). It is best to declare the variable that receives the return value as size_t .
Your function creates a copy of the input string with malloc() . This might be a requirement of the implementation, if so, disregard this item. Allocating dynamic memory is a costly operation, so high performance software should avoid it as much as possible. Plus, you place the burden of deallocating the buffer on the caller. Which is more likely to lead to memory leaks. A best option would be perhaps to receive a null terminated string and reverse it in-place. I once wrote an in-place string reversing function that used the XOR swap algorithm. If you would like to compare it against yours here it is:
char * StringReverse(char * str)
{
assert(str != NULL);
if (!*str)
{
/* Empty string, do nothing */
return (str);
}
char * p1;
char * p2;
for (p1 = str, p2 = str + strlen(str) - 1; p2 > p1; ++p1, --p2)
{
/* XOR trick to swap integer values: */
*p1 ^= *p2;
*p2 ^= *p1;
*p1 ^= *p2;
}
return (str);
}
Unlike yours, this function will reverse the input string in-place, without malloc() . You can also easily write a wrapper function that mallocs a new string, passes it to StringReverse() and then returns it. The XOR swapping might or might not be faster, this will largely depend on the CPU architecture.
Extraneous call to strlen() . Bare in mind that a C string, unlike its C++ std::string counterpart, does not carry extra information about its length, except for the null char that terminates it. So each call to strlen() is a full scan of the string to find out its length (count the chars til a null is found). You can easily speed up your code by calling strlen() once and saving the result:
char *mon_string_rev(char *string)
{
assert(string != NULL)
size_t input_len = strlen(string);
char *out = malloc(input_len + 1);
assert(out != NULL);
// use 'input_len' as many times as it is needed.
....
Also note that strlen() returns a size_t object (which is usually a typedef to unsigned long ). It is best to declare the variable that receives the return value as such.
Your function creates a copy of the input string with malloc() . This might be a requirement of the implementation, if so, disregard this item. Allocating dynamic memory is a costly operation, so high performance software should avoid it as much as possible. Plus, you place the burden of deallocating the buffer on the caller. Which is more likely to lead to memory leaks. A best option would be perhaps to receive a null terminated string and reverse it in-place. I once wrote an in-place string reversing function that used the XOR swap algorithm. If you want to compare it against yours here it is:
char * StringReverse(char * str)
{
assert(str != NULL);
if (!*str)
{
/* Empty string, do nothing */
return (str);
}
char * p1;
char * p2;
for (p1 = str, p2 = str + strlen(str) - 1; p2 > p1; ++p1, --p2)
{
/* XOR trick to swap integer values: */
*p1 ^= *p2;
*p2 ^= *p1;
*p1 ^= *p2;
}
return (str);
}
Unlike yours, this function will reverse the input string in-place, without malloc() . You can also easily write a wrapper function that mallocs a new string, passes it to StringReverse() and then returns it. The XOR swapping might or might not be faster, this will largely depend on the CPU architecture though.
Extraneous call to strlen() . Bare in mind that a C string, unlike its C++ std::string counterpart, does not carry extra information about its length, except for the null char that terminates it. So each call to strlen() is a full scan of the string to find out its length (count the chars til a null is found). You can easily speed up your code by calling strlen() once and saving the result:
char *mon_string_rev(char *string)
{
assert(string != NULL)
size_t input_len = strlen(string);
char *out = malloc(input_len + 1);
assert(out != NULL);
// use 'input_len' as many times as it is needed.
....
Also note that strlen() returns a size_t object (which is usually a typedef to unsigned long ). It is best to declare the variable that receives the return value as size_t .
Your function creates a copy of the input string with malloc() . This might be a requirement of the implementation, if so, disregard this item. Allocating dynamic memory is a costly operation, so high performance software should avoid it as much as possible. Plus, you place the burden of deallocating the buffer on the caller. Which is more likely to lead to memory leaks. A best option would be perhaps to receive a null terminated string and reverse it in-place. I once wrote an in-place string reversing function that used the XOR swap algorithm. If you would like to compare it against yours here it is:
char * StringReverse(char * str)
{
assert(str != NULL);
if (!*str)
{
/* Empty string, do nothing */
return (str);
}
char * p1;
char * p2;
for (p1 = str, p2 = str + strlen(str) - 1; p2 > p1; ++p1, --p2)
{
/* XOR trick to swap integer values: */
*p1 ^= *p2;
*p2 ^= *p1;
*p1 ^= *p2;
}
return (str);
}
Unlike yours, this function will reverse the input string in-place, without malloc() . You can also easily write a wrapper function that mallocs a new string, passes it to StringReverse() and then returns it. The XOR swapping might or might not be faster, this will largely depend on the CPU architecture.
|
|
|
2
|
|
|
Just for the record, since you are interested Focusing on performance improvements, I once wrote a string reversing function there are at least two main improvement points in your code:
Extraneous call to strlen() . Bare in mind that a C string, unlike its C++ std::string counterpart, does not carry extra information about its length, except for the null char that terminates it. So each call to strlen() is a full scan of the string to find out its length (count the chars til a null is found). You can easily speed up your code by calling strlen() once and saving the result:
char *mon_string_rev(char *string)
{
assert(string != NULL)
size_t input_len = strlen(string);
char *out = malloc(input_len + 1);
assert(out != NULL);
// use 'input_len' as many times as it is needed.
....
Also note that used the XOR swap algorithm. If you wantstrlen() returns a size_t object (which is usually a typedef to compare it against yours here it unsigned long ). It is: best to declare the variable that receives the return value as such. char * StringReverse(char * str)
{
assert(str != NULL);
if (!*str)
{
/* Empty string, do nothing */
return (str);
}
char * p1;
char * p2;
for (p1 = str, p2 = str + strlen(str) - 1; p2 > p1; ++p1, --p2)
{
/* XOR trick to swap integer values: */
*p1 ^= *p2;
*p2 ^= *p1;
*p1 ^= *p2;
}
return (str);
}
Your function creates a copy of the input string with malloc() . This might be a requirement of the implementation, if so, disregard this item. Allocating dynamic memory is a costly operation, so high performance software should avoid it as much as possible. Plus, you place the burden of deallocating the buffer on the caller. Which is more likely to lead to memory leaks. A best option would be perhaps to receive a null terminated string and reverse it in-place. I once wrote an in-place string reversing function that used the XOR swap algorithm. If you want to compare it against yours here it is:
char * StringReverse(char * str)
{
assert(str != NULL);
if (!*str)
{
/* Empty string, do nothing */
return (str);
}
char * p1;
char * p2;
for (p1 = str, p2 = str + strlen(str) - 1; p2 > p1; ++p1, --p2)
{
/* XOR trick to swap integer values: */
*p1 ^= *p2;
*p2 ^= *p1;
*p1 ^= *p2;
}
return (str);
}
Unlike yours, this function will reverse the input string in-place, without malloc() . You can also easily write a wrapper function that mallocs a new string, passes it to StringReverse() and then returns it. The XOR swapping might or might not be faster, this will largely depend on the CPU architecture though.
Just for the record, since you are interested on performance improvements, I once wrote a string reversing function that used the XOR swap algorithm. If you want to compare it against yours here it is: char * StringReverse(char * str)
{
assert(str != NULL);
if (!*str)
{
/* Empty string, do nothing */
return (str);
}
char * p1;
char * p2;
for (p1 = str, p2 = str + strlen(str) - 1; p2 > p1; ++p1, --p2)
{
/* XOR trick to swap integer values: */
*p1 ^= *p2;
*p2 ^= *p1;
*p1 ^= *p2;
}
return (str);
}
Unlike yours, this function will reverse the input string in-place. The XOR swapping might or might not be faster, this will largely depend on the CPU architecture though.
Focusing on performance improvements, there are at least two main improvement points in your code:
Extraneous call to strlen() . Bare in mind that a C string, unlike its C++ std::string counterpart, does not carry extra information about its length, except for the null char that terminates it. So each call to strlen() is a full scan of the string to find out its length (count the chars til a null is found). You can easily speed up your code by calling strlen() once and saving the result:
char *mon_string_rev(char *string)
{
assert(string != NULL)
size_t input_len = strlen(string);
char *out = malloc(input_len + 1);
assert(out != NULL);
// use 'input_len' as many times as it is needed.
....
Also note that strlen() returns a size_t object (which is usually a typedef to unsigned long ). It is best to declare the variable that receives the return value as such.
Your function creates a copy of the input string with malloc() . This might be a requirement of the implementation, if so, disregard this item. Allocating dynamic memory is a costly operation, so high performance software should avoid it as much as possible. Plus, you place the burden of deallocating the buffer on the caller. Which is more likely to lead to memory leaks. A best option would be perhaps to receive a null terminated string and reverse it in-place. I once wrote an in-place string reversing function that used the XOR swap algorithm. If you want to compare it against yours here it is:
char * StringReverse(char * str)
{
assert(str != NULL);
if (!*str)
{
/* Empty string, do nothing */
return (str);
}
char * p1;
char * p2;
for (p1 = str, p2 = str + strlen(str) - 1; p2 > p1; ++p1, --p2)
{
/* XOR trick to swap integer values: */
*p1 ^= *p2;
*p2 ^= *p1;
*p1 ^= *p2;
}
return (str);
}
Unlike yours, this function will reverse the input string in-place, without malloc() . You can also easily write a wrapper function that mallocs a new string, passes it to StringReverse() and then returns it. The XOR swapping might or might not be faster, this will largely depend on the CPU architecture though.
|
|
|
1
|
|
answered Aug 4 '14 at 2:10
|
Just for the record, since you are interested on performance improvements, I once wrote a string reversing function that used the XOR swap algorithm. If you want to compare it against yours here it is:
char * StringReverse(char * str)
{
assert(str != NULL);
if (!*str)
{
/* Empty string, do nothing */
return (str);
}
char * p1;
char * p2;
for (p1 = str, p2 = str + strlen(str) - 1; p2 > p1; ++p1, --p2)
{
/* XOR trick to swap integer values: */
*p1 ^= *p2;
*p2 ^= *p1;
*p1 ^= *p2;
}
return (str);
}
Unlike yours, this function will reverse the input string in-place. The XOR swapping might or might not be faster, this will largely depend on the CPU architecture though.
|
|