[boost] Re: Singleton: review

From: Pavel Vozenilek (pavel_vozenilek_at_[hidden])
Date: 2005-05-16 06:29:02

"Gennadiy Rozental" wrote:

> I know I am slightly late but I am quite busy lately.
>
You are much welcomed. Thanks for the time to
do the review and all the details.

> Problem domain
> ============
>
> But... from my experience it's really bad idea to try to manage an order 
> of destruction of global variables. Any responsible library designers 
> should prefer explicit teardown procedure.
Explicit teardowns would be source of coupling and something
pretty hard to maintain (for me, others may find it easy).

>  * it doesn't seem to be ever portable to rely on order of global 
> variables destruction in different libraries (dynamic or static)
>
Singletons in DLLs are pretty hard problem and standard C++
doesn't seem to provide enough means to handle them.

One solution considered is to use unique shared memory
where named singletons get registered but this is still huge work ahead.

> Design
> =====
>
> PBD is not about supplying implementation as template parameter and any 
> kind of "super policies" are never a good idea IMO. They trying to bring 
> simplicity of single template parameter, but does not really bring 
> anything. In trivial cases (use all defaults) design with orthogonal 
> polices is as simple. While any customization attempt immediately shows an 
> advantage of later is more convenient and straightforward. For example if 
> I want to use custom locking policy with submitted singleton I would 
> write:
>
> class A : singleton_ex<static_lifetime_ex<instant_creation,my_lock> > 
> {...};
>
> Here I not only need to mention default creation policy, but also strange 
> beast static_lifetime_ex. And I only needed custom lock:
>
> class A : singleton<lock<my_lock> > {}
>
This is valid issue. Perhaps named template parameters technique
could be used here.

> Inability of the author to clearly recognize orthogonal policies lead to 2 
> subpolicies with "create" in their name: CreationPolicy and Creator.
The name and concept may be reconsidered as suggested
by Dan Rosen in other thead.

> I do not see why would anyone would need to employ malloc or 
> std::allocator to allocate memory for global variable.
Quite often. If you do custom memory management with new/delete
you often set it up after statics initialization (say when loading a DLL).

Either one needs to do nasty tricks with shared variables or use malloc/etc.

> Most interesting is that among all of these "creators"/creation_policies I 
> don't see a solution for singleton with non-default constructor.
>
Yes, this should be supported.

It is possible to explicitly pass parameters to the constructor as:
  A:pointer ptr;
  ptr->create(111);
but this is different idiom and different use case.

> -----------------
>
> Singletons very frequently are the bottleneck of program performance. Any 
> singleton solution couldn't afford significant performance overhead. 
> Submitted design clearly leads to some rotationally inlined functions to 
> be offlined (in addition to extra overhead). For the following trivial 
> class
>
> class A {
> public:
>    void foo() {++i; }
> private:
>    int i;
> };
>
> I noticed that
>
> A* a = new A;
>
> int n = ..;
> while( n-- > 0)
>  a->foo();
>
> is about 300 times faster then (modify definition above to employ default 
> singleton: class A : public basic_singleton<A>)
>
> (used g++ 3.3.3 on windows)
>

I tried this with Intel 7 plugged in VC6 IDE, Release mode,
all settings default and the ratio was 8.7 (in favor of raw class A).

> -----------------
>
> Author does admit that multithreading issues were not considered. IMO any 
> singleton design that doesn't cover MT couldn't be accepted as generic.
Not every library in Boost is MT safe even when it could be useful.
MT was left into next stage.

> Interface
> ======
>
> * A::pointer interface is not convenient. I do not want to know about any 
> "pointer". It should be either A::instance() or even better simply A;
>
Yes. Member function

T::pointer instance() {
   T::pointer ptr;
   return ptr;
}

has been suggested. This would allow the A::instance() syntax.

It is also possible to wrap the singleton class into
monostate like class:

struct AA
{
   A::pointer ptr;
   void DoSomething() { ptr->DoSomething(); }
};

I suggested it as example but shortage of time ...

/Pavel