Singletons

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Singletons

Jordi Gutiérrez Hermoso-2
Sorry for feeling argumentative today, but Octave activity has been a
little low lately, so I don't feel too bad about livening it up.

2011/5/25 John W. Eaton <[hidden email]>:

> On 25-May-2011, Jordi Gutiérrez Hermoso wrote:
>
> | I've never seen that place. A namespaced global object works just as
> | well and has less syntactic cruft built around it.
>
> It does not work just as well when a static object is involved.  Then
> you have to somehow be sure that the initialization happens at the
> right time, and that can be problematic because you have no control
> over the order of initialization of file-scope or global static
> objects.

Well, as an example that I got curious about from your list further
below, from reading the source of src/DLD-FUNCTIONS/fftw.cc and the
associated liboctave/oct-fftw.* files, I see that initialisation is
right now done at the moment when octave_fftw_planner::method() is
first called (which calls instance_ok which in turn attempts the
initialisation). It would in fact be relatively easy to change this
code to simply namespace the global octave_fftw_planner object; the
calling code in fftw.cc is in fact only using the class aspects of the
singleton as namespacing. And the dtor is never called; it's in fact
protected and the class is never derived from (and you would probably
run into problems if you tried to subclass it).

The point is that a singleton is antithetical to OOP because it's not
really a class with instantiated objects; it's just a global object
with global functions. Singletons are procedural programming with the
keywords "class" and "static" attached to them. You can't usually
derive from them (hence they don't enable polymorphism), you can't
instantiate several of their kind, so the actual benefits of OOP
aren't there with singletons.

But as I said, it's mostly about style. A singleton is just about
writing global objects and functions with the "class" and "static"
keywords; it's about de-OOPifying a class. This is for example
essential in Java where you can't have code outside of a class; there
you really have no choice but to use singletons if you don't want OOP.
It is a matter of style if you still prefer to write singletons in C++
instead of using namespaces.

> We've seen this kind of problem before where Octave crashes
> at startup on some systems and not others due to some object being
> instantiated in an order that we didn't expect.

We could be explicit about this order by calling an init() function
from main(), for objects that really need to exist for the entire
duration of execution.

> | This is overall a minor stylistic point, though. If everyone else
> | agrees that a singleton profiler is really the way to go, I will
> | relent. There is the issue of memory management, but we already
> | have a bunch of minor "memory leaks" (mostly allocations of small
> | global objects that last for the entire duration of execution and
> | are never deleted) that one more probably won't be noticed.
>
> If they are global objects that may be used at any time during the
> execution of the program and their expected lifetime is the entire
> lifetime of the program, then I don't see that these are really
> memory leaks.  Maybe that's what you meant to imply with your
> quotation marks?

Yes, that's what I meant, most of these are not really memory leaks
because they really are needed throughout the entire execution, so
valgrind is just a little cautious because free() is never called on
them, but...

> Here is a list of all the singleton objects in Octave that I could
> find (I grepped the .cc files for "instance = 0") and that might give
> rise to these "memory leaks":
>
>  path searching class
>  command history
>  fftw planner info
>  file directory separator character info
>  sparse matrix parameters
>  machine info
>  random number generator state
>  execution environment info
>  command editor
>  debugging breakpoint table
>  font manager
>  comment buffer for parser
>  interpreter call stack
>  symbol table info
>  list of dynamically loaded functions
>  list of figure objects
>  function load path info
>  list of graphics handles
>  graphics display info
>  pager output stream
>  diary output stream
>  list of other i/o streams
>  list of child processes

Some of these shouldn't really exist for the entire execution, right?
The fftw stuff should be deleted when Octave is done working with fft;
same if all sparse matrices go out of scope, as well as graphical
stuff. Is this correct? I think all of these are small, though, so
they're not really noticeable.

Not that I think it's worth the trouble "fixing" this. Not broke,
don't fix it. But I do think it's not necessary to introduce more
elements to that list unless it's essential, and it's not clear to me
that a profiler should necessarily belong to this list.

- Jordi G. H.
Reply | Threaded
Open this post in threaded view
|

Singletons

John W. Eaton
Administrator
On 25-May-2011, Jordi Gutiérrez Hermoso wrote:

| Well, as an example that I got curious about from your list further
| below, from reading the source of src/DLD-FUNCTIONS/fftw.cc and the
| associated liboctave/oct-fftw.* files, I see that initialisation is
| right now done at the moment when octave_fftw_planner::method() is
| first called (which calls instance_ok which in turn attempts the
| initialisation). It would in fact be relatively easy to change this
| code to simply namespace the global octave_fftw_planner object; the
| calling code in fftw.cc is in fact only using the class aspects of the
| singleton as namespacing. And the dtor is never called; it's in fact
| protected and the class is never derived from (and you would probably
| run into problems if you tried to subclass it).
|
| [...]
|
| Some of these shouldn't really exist for the entire execution, right?
| The fftw stuff should be deleted when Octave is done working with fft;
| same if all sparse matrices go out of scope, as well as graphical
| stuff. Is this correct?

Not necessarily.  For example, the fft planner holds state about how
to compute the next FFT calculation, right?  So it has to stay around
as long as there is some possibility that fftw will be called again.
Likewise for spparms and graphics state, fonts, command editor and
history, etc.  So as far as I can tell, these things never go out of
scope.

But again, if you think that something could be improved, propose a
patch.

jwe
Reply | Threaded
Open this post in threaded view
|

Singletons

John W. Eaton
Administrator
In reply to this post by Jordi Gutiérrez Hermoso-2
On 25-May-2011, Jordi Gutiérrez Hermoso wrote:

| Sorry for feeling argumentative today, but Octave activity has been a
| little low lately, so I don't feel too bad about livening it up.

I can be argumentative too, but I'd rather play nice and I'd rather
not waste what little time I have to work on Octave at the moment
arguing about these kinds of things unless you have patches that you'd
like to propose which will actually improve something.

| It would in fact be relatively easy to change this
| code to simply namespace the global octave_fftw_planner object; the
| calling code in fftw.cc is in fact only using the class aspects of the
| singleton as namespacing. And the dtor is never called; it's in fact
| protected and the class is never derived from (and you would probably
| run into problems if you tried to subclass it).

Yes, you could do this but you'd still have to initialize the global
object somewhere and it seems better to me to have a container of
sorts (the singleton) that manages that instead of having a separate
init function and requiring a user to call that.  But maybe I'm
missing some magic bullet here, and you could use this particular case
to show how we can do things better.  But in this case the destructor
will still never be called unless you want to create a mechanism for
things like this so that all these singleton objects will be cleaned
up in Octave's clean_up_and_exit function.  Of course we could do that
with the current singleton approach simply by adding a function like

  static void SINGLETON::cleanup (void)
  {
    delete instance;
  }

to every singleton class, then adding something like

  cleanup_functions::add (SINGLETON::cleanup);

to the SINGLETON::instance_ok function, in which
cleanup_functions::add places the SINGLETON::cleanup function in a list
of functions to be called just before Octave exits.  This might
be interesting to try and see what effect it has on the amount of lost
memory that valgrind reports.

(Look, cleanup_functions could itself be a singleton!)

OTOH, we do have other classes which do use static class members as a
way of implementing namespaces.  That was done because those things
were written before namespaces were universally supported by C++
compilers.  It would be nice to fix those to use namespaces instead,
but that fix should probably be done when we think about putting all
of Octave inside a (set of) namespace(s).

| The point is that a singleton is antithetical to OOP because it's not

OK, so it is not OOP.  There's nothing that says that a program has to
be slavishly OO, is there?  As I said, I see benefits to using the
singleton class when there is global data to manage.  I don't think we
have over used this idiom in a bad way.

jwe
Reply | Threaded
Open this post in threaded view
|

Re: Singletons

jacob.dawid
Singletons are just fine in OOP. It's perfectly okay to have a unique object that offers a public interface. A singleton is not just a "global class with a global interface", this is just not true.