copy constructors hurting performance

[Shea Martin]

I have a String class (I know I am re-inventing the wheel, yes I have
heard of boost, and of QString).



My copy constructor does a deep (strcpy) of the char *_buffer member.
I have a member function func(const String &param). When an actual
String is passed as the param this is nice and efficient.

I have a constructor which takes a const char* as an argument. And
performs a deep copy of the const char *buffer.

The issue occurrs when I make the following call:
func("this is the param");
The String(const char *buf) constructor is called, making a deep copy of
the data. Because func takes a const arg, it will never alter the
buffer, thus the deep copy of the const char * is needless, and could be
expensive.

Does anyone see an easy and somewhat safe design, to avoid the needless
copy's? Or is the only way to overload the methods to take a const
char* as well?

I think I already know the answer to this, but I would rather be safe
that sorry.

Thanks,

~S

 

[Ron Natalie]

The String(const char *buf) constructor is called, making a deep copy of
the data. Because func takes a const arg, it will never alter the
buffer, thus the deep copy of the const char * is needless, and could be
expensive.

It may not be needless. What guarantees do you have that the char* passed
to the constructor remains valid over the lifetime of the object. Imagine this:

String* string_ptr;
{
char arg[] = "Transient string."
string_ptr = new String(arg);
}
// at this point string_ptr if it just saves the pointer has a pointer to deallocated
// memory.

Does anyone see an easy and somewhat safe design, to avoid the needless
copy's? Or is the only way to overload the methods to take a const
char* as well?

Overloading has no bearing on the problem.

Why are you reimplementing the wheel anyhow?

 

[Victor Bazarov]

I have a String class (I know I am re-inventing the wheel, yes I have
heard of boost, and of QString).



My copy constructor does a deep (strcpy) of the char *_buffer member.
I have a member function func(const String &param). When an actual
String is passed as the param this is nice and efficient.

I have a constructor which takes a const char* as an argument. And
performs a deep copy of the const char *buffer.

The issue occurrs when I make the following call:
func("this is the param");
The String(const char *buf) constructor is called, making a deep copy of
the data. Because func takes a const arg, it will never alter the
buffer, thus the deep copy of the const char * is needless, and could be
expensive.

Does anyone see an easy and somewhat safe design, to avoid the needless
copy's? Or is the only way to overload the methods to take a const
char* as well?

I think I already know the answer to this, but I would rather be safe
that sorry.

IMHO, what you're looking for is a variation on reference-counting
implementation. If you know that the string will never change, you
could simply store the pointer instead of making a deep copy. I has
to make a copy only when a mutating member function is called.

It is, however, dangerous. The pointer you store may simply become
invalid:

char *ptr = new char[20];
strcpy(ptr, "whatever");
String str(ptr);
delete[] ptr; // BOOM! the ptr is invalid, 'str' cannot use it
// safely any longer.

However, in the case of creating a temporary String from a string
literal, it will work.

Victor

 

[David Rubin]

Shea Martin wrote:

[snip - writing String class]

My copy constructor does a deep (strcpy) of the char *_buffer member.
I have a member function func(const String &param). When an actual
String is passed as the param this is nice and efficient.

I have a constructor which takes a const char* as an argument. And
performs a deep copy of the const char *buffer.

The issue occurrs when I make the following call:
func("this is the param");
The String(const char *buf) constructor is called, making a deep copy of
the data. Because func takes a const arg, it will never alter the
buffer, thus the deep copy of the const char * is needless, and could be
expensive.

I think you want to define

void func(const char *buf);

and overload for const String&:

// pass the char* representation
void func(const String& s) { func(s.rep); }

This avoids calling the copy constructor. You can further protect
against this unwanted conversion by making the String constructor
'explicit':

explicit String(const char* s);

/david

 

[Shea Martin]

It may not be needless. What guarantees do you have that the char* passed
to the constructor remains valid over the lifetime of the object. Imagine this:

String* string_ptr;
{
char arg[] = "Transient string."
string_ptr = new String(arg);
}
// at this point string_ptr if it just saves the pointer has a pointer to deallocated
// memory.

I am well aware of the this danger, that is why I do make a deep copy.
That is why I posted here, to see if anyone knew of a different solution.

Overloading has no bearing on the problem.

Excuse me?
int main()
{
func("print this please"); //String::String(const char *text) called
String str("print this");
func(str); //no constructor called, hence no mem alloc, and copy
return 0;
}

void func(const String &text)
{
printf("String version\ntext is %s\n", text.cstr());
}

no if I add this function (btw this is called function overloading), I
avoid the use of String::String(const char *text):

String::String(const char *text)
{
internalBuffer = new char[strlen(text)+1];
strcpy(internalBuffer, text);
}

Why are you reimplementing the wheel anyhow?

My managers are anti STL (and to be honest debugging code with STL in it
is ugly, due to the templates, atleast w/ our debugger, dbx). The one
does beleives that a class should never be used, but luckily the other
does uses classes. My managers also have a phobia of 3rd party stuff,
such as boost or QString. They are old-school. I have no control over
this, and I like my paycheck, so I listen. I have a bunch of code I
wrote using QString and std::string: they want me to remove all of the
QString and STL stuff from it. Thus I would like a drop in replacement
for the two. It is not like it is hard to make a String class. It only
took me a day to may a replacement which covers all the functionality of
std::string, and 75% of QString. I would just like to make it a little
more efficient.

~S

 

[Shea Martin]

I have a String class (I know I am re-inventing the wheel, yes I have
heard of boost, and of QString).



My copy constructor does a deep (strcpy) of the char *_buffer member.
I have a member function func(const String &param). When an actual
String is passed as the param this is nice and efficient.

I have a constructor which takes a const char* as an argument. And
performs a deep copy of the const char *buffer.

The issue occurrs when I make the following call:
func("this is the param");
The String(const char *buf) constructor is called, making a deep copy of
the data. Because func takes a const arg, it will never alter the
buffer, thus the deep copy of the const char * is needless, and could be
expensive.

Does anyone see an easy and somewhat safe design, to avoid the needless
copy's? Or is the only way to overload the methods to take a const
char* as well?

I think I already know the answer to this, but I would rather be safe
that sorry.

IMHO, what you're looking for is a variation on reference-counting
implementation. If you know that the string will never change, you
could simply store the pointer instead of making a deep copy. I has
to make a copy only when a mutating member function is called.

It is, however, dangerous. The pointer you store may simply become
invalid:

char *ptr = new char[20];
strcpy(ptr, "whatever");
String str(ptr);
delete[] ptr; // BOOM! the ptr is invalid, 'str' cannot use it
// safely any longer.

However, in the case of creating a temporary String from a string
literal, it will work.

Victor

I know. I was thinking that it would be nice to be able to overload a
constructor based on whether it was a string literal or not... Or even
better, remove char * from this universe, that would solve the issue
too; a la Java. Unfortunately, have to remain compatible with rest of
world.

Thanks,

~S

 

[Shea Martin]

Shea Martin wrote:

[snip - writing String class]

My copy constructor does a deep (strcpy) of the char *_buffer member.
I have a member function func(const String &param). When an actual
String is passed as the param this is nice and efficient.

I have a constructor which takes a const char* as an argument. And
performs a deep copy of the const char *buffer.

The issue occurrs when I make the following call:
func("this is the param");
The String(const char *buf) constructor is called, making a deep copy
of the data. Because func takes a const arg, it will never alter the
buffer, thus the deep copy of the const char * is needless, and could
be expensive.

I think you want to define

void func(const char *buf);

and overload for const String&:

// pass the char* representation
void func(const String& s) { func(s.rep); }

This avoids calling the copy constructor. You can further protect
against this unwanted conversion by making the String constructor
'explicit':

explicit String(const char* s);

/david

That is what I thought, I was just being lazy, hoping for a dif sol'n.
However, I am unfamiliar with the 'explicit' keyword. I am on my way
over to the book shelf to look it up.

Thanks,

~S

 

[Ron Natalie]

Excuse me?

Just what I said. Overloading a const char* version of the constructor
doesn't have any real outward effects (other than allowing you to initialize
from const char*).

String::String(const char *text)
{
internalBuffer = new char[strlen(text)+1];
strcpy(internalBuffer, text);
}

So why is this any different than the String(char*) case?

My managers are anti STL (and to be honest debugging code with STL in it
is ugly, due to the templates, atleast w/ our debugger, dbx).

Your managers are ignorant fools. Get a better debugger or learn to deal
with it. std::string is not STL, it's not some third-party add-on. It is an
integral part of the standard C++ language.

The one
does beleives that a class should never be used, but luckily the other
does uses classes.

As I said, they appear to be ignorant fools.

They are old-school. I have no control over
this, and I like my paycheck, so I listen. I have a bunch of code I
wrote using QString and std::string: they want me to remove all of the
QString and STL stuff from it. Thus I would like a drop in replacement
for the two. It is not like it is hard to make a String class. It only
took me a day to may a replacement which covers all the functionality of
std::string, and 75% of QString. I would just like to make it a little
more efficient.

Great, and now you must maintain it, and I warrant you haven't discovered
all the problems with it, and six months from now when they want you to use
wchar_t because you've added unicode you'll have to recode it.

 

[Ron Natalie]

I know. I was thinking that it would be nice to be able to overload a
constructor based on whether it was a string literal or not... Or even
better, remove char * from this universe, that would solve the issue
too; a la Java. Unfortunately, have to remain compatible with rest of
world.

const char* doesn't tell you if it is a string literal or not. You can have
const char* values that aren't string literals.

 

[Cagdas Ozgenc]

It is, however, dangerous. The pointer you store may simply become
invalid:

char *ptr = new char[20];
strcpy(ptr, "whatever");
String str(ptr);
delete[] ptr; // BOOM! the ptr is invalid, 'str' cannot use it
// safely any longer.

However, in the case of creating a temporary String from a string
literal, it will work.

I think this is not a good illustration, how about

in some header file:

typedef char *String;

in implementation file:

char *ptr = new char[20];
strcpy(ptr,"hello");

String str = ptr; // illusion of copy constructor
delete[] ptr; // BOOM

As you can see this is problematic only when the deletion is in another
scope. Within the same scope it is programmer's responsibility not to hack
the branch he is sitting on.

 

[Ron Natalie]

Do some O/S compiler specific stuff and determine whether arg and
this pointer points to stack or free store

There's also statically allocated objects in addition to stack (auto) and free store.

But none of this helps either. What happens if someone changes characters
referred to by the passed in pointer and you haven't done the deep copy.

char foo[] = "yada yada"

String foostring(foo);

strcpy(foo, "daba daba");

Now what do you expect the value of foostring to be?

 

[Cagdas Ozgenc]

In contrast to what others said, I have a feeling that what you dream of may
be achieved with delicacy, but I would never do it myself.

First of all, you need to be able to detect whether an object is sitting on
the stack or free store:

class String {
public:
String(char const * const arg) {
Do some O/S compiler specific stuff and determine whether arg and
this pointer points to stack or free store
If arg is on free store do a deep copy. If arg is on stack, do NOT
do deep copy if this object is on stack otherwise do deep copy.
}

String(String const& arg) {
Do some O/S compiler specific stuff and determine whether &arg and
this pointer points to stack or free store
If arg is on free store do a deep copy. If arg is on stack, do NOT
do deep copy if this object is on stack otherwise do deep copy.
}
};


I suppose according to the scope rules a programmer should respect, the
above, not carefully thought, highly dangerous implementation might do the
trick. It's just an idea...

 

[Shea Martin]

Just what I said. Overloading a const char* version of the constructor
doesn't have any real outward effects (other than allowing you to initialize
from const char*).

I meant overload the functions which take a const String reference, and
man make a version that takes a const char* as well. Then when these
methods are called, no constructor is called.

String::String(const char *text)
{
internalBuffer = new char[strlen(text)+1];
strcpy(internalBuffer, text);
}

So why is this any different than the String(char*) case?

My managers are anti STL (and to be honest debugging code with STL in it
is ugly, due to the templates, atleast w/ our debugger, dbx).

Your managers are ignorant fools. Get a better debugger or learn to deal
with it. std::string is not STL, it's not some third-party add-on. It is an
integral part of the standard C++ language.

The one
does beleives that a class should never be used, but luckily the other
does uses classes.

As I said, they appear to be ignorant fools.

They are old-school. I have no control over
this, and I like my paycheck, so I listen. I have a bunch of code I
wrote using QString and std::string: they want me to remove all of the
QString and STL stuff from it. Thus I would like a drop in replacement
for the two. It is not like it is hard to make a String class. It only
took me a day to may a replacement which covers all the functionality of
std::string, and 75% of QString. I would just like to make it a little
more efficient.

Great, and now you must maintain it, and I warrant you haven't discovered
all the problems with it, and six months from now when they want you to use
wchar_t because you've added unicode you'll have to recode it.

I don't think we will need unicode anytime soon, but point taken. I am
well aware of the downfall of reinventing, but am given little alternative.

What is your suggestion? March into their office and state "According
to Ron Natalie, you are ignorant fools. From this day forth, I shall be
using std::string. So take your char pointers and stuff up your royal
arses! I'll be in my office if you need me."

Basically I have 2 options, roll my own, or use char*. Maybe I am
oversciencing this, maybe I should just use char*? I have chewed on
this decision for a while.

It is easy to be a critic, if you don't have to have a solution.

~S

 

[Shea Martin]

As I said, they appear to be ignorant fools.

It gets worse:
Sun Compilers generate a disk cache when compiling STL code. My
managers had never seen it before I was hired. They panicked. They
want all code, that causes the SunWS_cache directory to be created,
removed. This includes my maps, list, cout, cerr, cin, ofstream,
ifstream**, etc. Whoa, is the life of a junior programmer.

~S

**: turns out that most of the fstream code has already been removed
anyway, due to the fact that we frequently deal with files larger than
2GB, which is a fstream limit. This limit is bypassed on Solaris by
compiling with -xarch=v9, but makes binaries which are incompatible with
32 bit libs (which we have to use.)

 

[Ron Natalie]

Basically I have 2 options, roll my own, or use char*. Maybe I am
oversciencing this, maybe I should just use char*? I have chewed on
this decision for a while.

Why don't you "roll your own" by taking an open source implementation of
std::string and using that? Feel free to change the leading s in string to
upper case.

 

[Ron Natalie]

Sun Compilers generate a disk cache when compiling STL code. My
managers had never seen it before I was hired.

It's not so much STL as what the compiler does with ANY template.

They panicked. They
want all code, that causes the SunWS_cache directory to be created,
removed.

You know you can just shut that off with a compiler option (-pto). Then it
does not share the instantiations of the templates between translation units.
Your program possibly gets a little fatter, as a result.

 

[lilburne]

I have a String class (I know I am re-inventing the wheel, yes I have
heard of boost, and of QString).



My copy constructor does a deep (strcpy) of the char *_buffer member.
I have a member function func(const String &param). When an actual
String is passed as the param this is nice and efficient.

I have a constructor which takes a const char* as an argument. And
performs a deep copy of the const char *buffer.

The issue occurrs when I make the following call:
func("this is the param");
The String(const char *buf) constructor is called, making a deep copy of
the data. Because func takes a const arg, it will never alter the
buffer, thus the deep copy of the const char * is needless, and could be
expensive.

Does anyone see an easy and somewhat safe design, to avoid the needless
copy's? Or is the only way to overload the methods to take a const
char* as well?

I think I already know the answer to this, but I would rather be safe
that sorry.

Actually we were reminiscing about this today. We built a
'String' class 10 years ago that did all sorts of clever
things to avoid deep copying string literals, sharing
between strings, optimized this, and optimized that. Unless
you actually forced it to take a copy of its argument the
thing was unusable and prone to causing the application to
crash. It wasn't untill we had purged the class of all its
clever nonesense that it became usable.

Why do you think you have a performance problem anyway?

 

[Paul McKenzie]

My managers are anti STL (and to be honest debugging code with STL in it
is ugly, due to the templates, atleast w/ our debugger, dbx).

std::string is not STL. Your managers are wrong.

My managers also have a phobia of 3rd party stuff,

std::string is not "third-party stuff". It is part of the core C++ standard
library. I guess your managers suggest you code your own FILE* routines
too, or are they more trusting in 'C' library routines?

I'm amazed at how many ignorant "C++ phobes" won't touch anything that may
suggest usage of a template or class from the standard C++ library because,
well, it's C++, but never hesitate to use fopen(), strpcy(), memset(),
qsort(), sqrt() or any other function such as these. Go figure.

such as boost or QString. They are old-school. I have no control over
this, and I like my paycheck, so I listen. I have a bunch of code I
wrote using QString and std::string: they want me to remove all of the
QString and STL stuff from it.

If you leave std::string alone, you can tell them that you did remove all
the QString and STL stuff, since std::string a'int STL

Paul

 

[Paul McKenzie]

It gets worse:
Sun Compilers generate a disk cache when compiling STL code. My
managers had never seen it before I was hired. They panicked. They
want all code, that causes the SunWS_cache directory to be created,
removed. This includes my maps, list, cout, cerr, cin, ofstream,
ifstream**, etc. Whoa, is the life of a junior programmer.

Geez, talk about ignorance.

The STL code is just templates -- maybe similar to the same code that you
would write if you didn't bother with STL and created your own classes.
Therefore the disk cache would still have existed anyway if you wrote your
own template classes and didn't utilize one stitch of STL. I guess that any
usage of templates, your own or STL, is off-limits.


If this is a commercial app you are developing, make sure you let me know
what the name of the application is. I need to be warned...

Paul

 

[Shea Martin]

Geez, talk about ignorance.

The STL code is just templates -- maybe similar to the same code that you
would write if you didn't bother with STL and created your own classes.
Therefore the disk cache would still have existed anyway if you wrote your
own template classes and didn't utilize one stitch of STL. I guess that any
usage of templates, your own or STL, is off-limits. You got it.


If this is a commercial app you are developing, make sure you let me know
what the name of the application is. I need to be warned...

LOL.
Don't worry, it is in house.

Paul

~S