 |
C++Talk.NET
C++ language newsgroups
|
| View previous topic :: View next topic |
| Author |
Message |
gelbeiche
Guest
|
 Posted: Mon Jul 10, 2006 3:13 am Post subject: static class variable allocated at heap |
 |
|
Hi,
a project I'm involved in has code in the form:
class toConnectionProvider
{
static std::map<QCString, toConnectionProvider *> *Providers;
....
}
A provider can register itself in the map:
void toConnectionProvider::addProvider(const QCString &provider)
{
checkAlloc();
Provider = provider;
(*Providers)[Provider] = this;
}
and there is a member function which checks if the map is allocated.
void toConnectionProvider::checkAlloc(void)
{
if (!Providers)
Providers = new std::map<QCString, toConnectionProvider *>;
}
I have the following questions:
- AFAIK Providers is never "deleted" until program exit and usually
I expect a new/delete pair for dynamic memory allocation.
So I guess the idiom above(static class variable allocated at heap)
is not kosher ?!
- Is the code above a candidate for refactoring ?
A quick change would be to convert
map<>* Providers;
to
map Providers;
Is it a essential improvement ?
- Another idea would be to make a separate class
toConnectionProviderManager which stores the providers in a container.
Is it a good idea ?
- What other solutions come to mind when considering such code ?
Thomas
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ] |
|
| Back to top |
|
 |
kanze
Guest
|
| Posted: Tue Jul 11, 2006 2:39 am Post subject: Re: static class variable allocated at heap |
|
|
gelbeiche wrote:
| Quote: | a project I'm involved in has code in the form:
class toConnectionProvider
{
static std::map<QCString, toConnectionProvider *> *Providers;
...
}
A provider can register itself in the map:
void toConnectionProvider::addProvider(const QCString &provider)
{
checkAlloc();
Provider = provider;
(*Providers)[Provider] = this;
}
and there is a member function which checks if the map is allocated.
void toConnectionProvider::checkAlloc(void)
{
if (!Providers)
Providers = new std::map<QCString, toConnectionProvider *>;
}
|
Wouldn't it be more nature to call it getProviderMap, and have
it return a pointer to the map?
| Quote: | I have the following questions:
- AFAIK Providers is never "deleted" until program exit and usually
I expect a new/delete pair for dynamic memory allocation.
So I guess the idiom above(static class variable allocated at heap)
is not kosher ?!
|
It depends. In this case, the map is really a singleton, and
it's usual for a singleton to never be destructed. (I tend to
refer to such objects as "pseudo-static" objects.)
| Quote: | - Is the code above a candidate for refactoring ?
A quick change would be to convert
map<>* Providers;
to
map Providers;
Is it a essential improvement ?
|
It depends. Using the function avoids order of initialization
issues if a static object registers itself. It may also cause
threading issues in a multithreaded environment.
Note that if you want destruction, you can make the map a local
static variable in its getter function.
| Quote: | - Another idea would be to make a separate class
toConnectionProviderManager which stores the providers in a container.
Is it a good idea ?
|
There's no fixed rule. If the map is simple, as it is here, I
generally go they way you've gone here, with a couple of static
member functions to handle registration, lookup, and if
necessary, managing the singleton object. If the map is not
just a standard class, and has significant behavior, I'll make a
separate class of it.
--
James Kanze GABI Software
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ] |
|
| Back to top |
|
|
Ulrich Eckhardt
Guest
|
| Posted: Tue Jul 11, 2006 2:39 am Post subject: Re: static class variable allocated at heap |
|
|
gelbeiche wrote:
| Quote: | class toConnectionProvider
{
static std::map<QCString, toConnectionProvider *> *Providers;
...
}
A provider can register itself in the map:
void toConnectionProvider::addProvider(const QCString &provider)
{
checkAlloc();
Provider = provider;
(*Providers)[Provider] = this;
}
and there is a member function which checks if the map is allocated.
void toConnectionProvider::checkAlloc(void)
{
if (!Providers)
Providers = new std::map<QCString, toConnectionProvider *>;
}
I have the following questions:
- AFAIK Providers is never "deleted" until program exit and usually
I expect a new/delete pair for dynamic memory allocation.
So I guess the idiom above(static class variable allocated at heap)
is not kosher ?!
|
Why, because some mechanic leak-checker calls it a memory leak? It isn't,
because you can call the code a hundred times and the memory usage doesn't
increase. In fact it is an optimisation, because why go through the map and
destroy things when the only thing that does is release storage that would
be reclaimed anyway when the program exits.
| Quote: | - Is the code above a candidate for refactoring ?
A quick change would be to convert
map<>* Providers;
to
map Providers;
Is it a essential improvement ?
|
You remove one heap allocation. You remove the check whether this allocation
already took place.
You add one static constructor to call. This happens always, not optionally
when it is used like before. You add one static destructor to call with all
the overhead as described above.
| Quote: | - Another idea would be to make a separate class
toConnectionProviderManager which stores the providers in a container.
Is it a good idea ?
|
Not worth the hassle, I daresay. Well, maybe for a multithreading-safe
version thereof. ;)
Uli
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ] |
|
| Back to top |
|
|
roods
Guest
|
| Posted: Tue Jul 11, 2006 2:40 am Post subject: Re: static class variable allocated at heap |
|
|
The only thing that I see is that you should look into using smart
pointers for Providers and w/in the map. I'd use either shared_ptr or
scoped_ptr depending on your needs.
If the objects in the map aren't being shared at all then perhaps they
shouldn't be pointers.
gelbeiche wrote:
| Quote: | Hi,
a project I'm involved in has code in the form:
class toConnectionProvider
{
static std::map<QCString, toConnectionProvider *> *Providers;
...
}
A provider can register itself in the map:
void toConnectionProvider::addProvider(const QCString &provider)
{
checkAlloc();
Provider = provider;
(*Providers)[Provider] = this;
}
and there is a member function which checks if the map is allocated.
void toConnectionProvider::checkAlloc(void)
{
if (!Providers)
Providers = new std::map<QCString, toConnectionProvider *>;
}
I have the following questions:
- AFAIK Providers is never "deleted" until program exit and usually
I expect a new/delete pair for dynamic memory allocation.
So I guess the idiom above(static class variable allocated at heap)
is not kosher ?!
- Is the code above a candidate for refactoring ?
A quick change would be to convert
map<>* Providers;
to
map Providers;
Is it a essential improvement ?
- Another idea would be to make a separate class
toConnectionProviderManager which stores the providers in a container.
Is it a good idea ?
- What other solutions come to mind when considering such code ?
Thomas
|
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ] |
|
| Back to top |
|
|
Thomas Maeder
Guest
|
| Posted: Tue Jul 11, 2006 2:41 am Post subject: Re: static class variable allocated at heap |
|
|
gelbeiche <info (AT) randspringer (DOT) de> writes:
| Quote: | a project I'm involved in has code in the form:
class toConnectionProvider
{
static std::map<QCString, toConnectionProvider *> *Providers;
...
}
A provider can register itself in the map:
void toConnectionProvider::addProvider(const QCString &provider)
{
checkAlloc();
Provider = provider;
(*Providers)[Provider] = this;
}
and there is a member function which checks if the map is allocated.
void toConnectionProvider::checkAlloc(void)
{
if (!Providers)
Providers = new std::map<QCString, toConnectionProvider *>;
}
I have the following questions:
- AFAIK Providers is never "deleted" until program exit and usually
I expect a new/delete pair for dynamic memory allocation.
So I guess the idiom above(static class variable allocated at heap)
is not kosher ?!
|
That depends on your definition of "kosher". If the only resource that
map locks is memory, and the operating system reuses memory a process
has allocated when the proces terminates, then there I don't see a
real problem.
| Quote: | - Is the code above a candidate for refactoring ?
A quick change would be to convert
map<>* Providers;
to
map Providers;
Is it a essential improvement ?
|
That again depends. It certainly makes the code simpler, so as a rule
of thumb, it's a good thing. On the other hand, every process will now
have such a map, even if it wouldn't need it; I can't tell whether
that is acceptable.
| Quote: | - Another idea would be to make a separate class
toConnectionProviderManager which stores the providers in a container.
Is it a good idea ?
|
Only if you are going to need it.
| Quote: | - What other solutions come to mind when considering such code ?
|
That depends on the problem you have with it.
I have a problem with the variable Provider in
toConnectionProvider::addProvider(). My solution would be to eliminate
it.
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ] |
|
| Back to top |
|
|
kanze
Guest
|
| Posted: Wed Jul 12, 2006 2:11 am Post subject: Re: static class variable allocated at heap |
|
|
Ulrich Eckhardt wrote:
[...]
| Quote: | - Is the code above a candidate for refactoring ?
A quick change would be to convert
map<>* Providers;
to
map Providers;
Is it a essential improvement ?
You remove one heap allocation. You remove the check whether
this allocation already took place.
You add one static constructor to call. This happens always,
not optionally when it is used like before. You add one static
destructor to call with all the overhead as described above.
|
You missed an important point: you loose any order of
initialization guarantees. If the map is to be used during the
initialization of other static variables, you definitely want to
pass by a function to ensure correct initialization before use.
Note that similar considerations may affect destruction; if the
map is used in the destructors of any static objects, avoiding
destruction may be a requirement: not destructing at all is
about the only way I know to ensure not destructing before last
use.
--
James Kanze GABI Software
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ] |
|
| Back to top |
|
|
Earl Purple
Guest
|
| Posted: Thu Jul 13, 2006 3:47 am Post subject: Re: static class variable allocated at heap |
|
|
gelbeiche wrote:
| Quote: | Hi,
a project I'm involved in has code in the form:
class toConnectionProvider
{
static std::map<QCString, toConnectionProvider *> *Providers;
...
}
A provider can register itself in the map:
void toConnectionProvider::addProvider(const QCString &provider)
{
checkAlloc();
Provider = provider;
(*Providers)[Provider] = this;
}
and there is a member function which checks if the map is allocated.
void toConnectionProvider::checkAlloc(void)
{
if (!Providers)
Providers = new std::map<QCString, toConnectionProvider *>;
}
I have the following questions:
- AFAIK Providers is never "deleted" until program exit and usually
I expect a new/delete pair for dynamic memory allocation.
So I guess the idiom above(static class variable allocated at heap)
is not kosher ?!
|
It's not a leak as such but you'll have a hard time debugging your
problem with purify as it will show up as a leak and you could have
trouble working out which ones are valid and which ones are not. I
personally prefer not to have the idiom above.
| Quote: | - Is the code above a candidate for refactoring ?
A quick change would be to convert
map<>* Providers;
to
map Providers;
Is it a essential improvement ?
|
Do not have a loose static instance because you cannot control
initialisation order. But the best solution in my opinion is to use a
function that has the static member and returns it by reference. Here
there is some control as the member will not be initialised until the
function is first called. If it needs other singletons to already exist
it will call their appropriate function and you'll be safe as long as
there are no circular references.
In addition, should any constructor throw an exception it will be
catchable.
Finally, in a multi-threaded environment, there should be no problems
with race conditions using a static member (compilers have to be
compliant with this though), whereas there might be using a static
pointer.
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ] |
|
| Back to top |
|
|
Earl Purple
Guest
|
| Posted: Thu Jul 13, 2006 3:51 am Post subject: Re: static class variable allocated at heap |
|
|
gelbeiche wrote:
| Quote: | A quick change would be to convert
map<>* Providers;
to
map Providers;
Is it a essential improvement ?
|
I should (further to my previous post) point out what can be one
possible flaw with having a function that creates a static instance and
returning a reference to it. But it is only a possible flaw and if it
doesn't apply then go ahead and use it.
The flaw is that although you now have control over the order of
construction of your singleton objects, you do not have any control
over their destruction.
An example of this could well be prevalent in your class: if libraries
are loaded with dlopen() and objects from them are added to your map,
and then later the libraries are closed with dlclose(), all the objects
in your map will be invalid. This won't be a problem unless your map
makes some attempt to access them, including trying to delete them.
(Maybe they are shared_ptrs). But if you are not controlling the
destuction order, you cannot determine what gets deleted first. You may
not consider it a major problem if your application seg-faults when it
is closing down anyway (I suppose you can always delete the core file)
but it's more of a "bug" than the leak-issue).
If no singleton relies on the existence of anything else to exist at
the time of its destruction then it is fine.
My own rule is generally to avoid singletons. Incredible how I was able
to eliminate so many singletons that it turned out didn't need to be
singletons after all. Actually there is one singleton - a table of
dynamically-opened libraries.
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ] |
|
| Back to top |
|
|
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
|
|
FAQ
Search
Memberlist
Usergroups
Register
Profile
Log in to check your private messages
Log in
