[PD] back to memory allocation troubles

[Krzysztof Czaja]

hi again,

Miller Puckette wrote:
...
> Although Krzysztof has shown me wrong about how Pd is using
> memory allocation...!

no no, I would not say that!  At least not without careful memory
profiling.  I finally managed to do some testing, but not enough to
make any `statements' (what I can only say is that the basic design
is beautifully logical and reading Pd code is a pleasure :-)

Please treat what follows as loose remarks which might need to be
trashed after closer inspection.  Also appologies for somewhat
technical subject -- if you do not like such talk, then trash this
right away...

Krzysztof

[Now the techie part:]

Generally speaking, the main source of memory (de/re)allocation
activity are `binbufs', especially the one pointed to by inbinbuf
static pointer (s_inter.c) used in parsing messages received from
the gui process.  Most of the time this constant creating-growing-
-shrinking-deleting of inbinbuf contents is not causing much harm,
since allocation sequence repeatedly reuse the same location or
alternate between two locations.  But whenever this sequence is
intertwined with object creation etc. there could be some danger
of memory fragmentation (how serious it is depends on os).

I have no idea how to limit this activity.  The binbuf manipulation
belongs to the very foundation of Pd and touching it has to be
risky.  A safer approach would be to define a nonshrinking (or
rarely shrinking) binbuf variant to be substituted for the most
active binbufs.  But then most of the routines in m_binbuf.c would
need to have nonshrinking versions.

One ad hoc improvement is probably to make a histogram of binbuf
sizes generated by binbuf_text() routine (after shrinking) and
choose the best (probably smaller then currently used size of
16 atoms?) initial value for nalloc variable.

During testing I also found a few memory leaks.  One -- the leaking
of local binbuf in save routine -- could partly explain the
save-close-open-crash mystery reported by Gregorio García and Greg
Rippin. This leakage can be quite large, depending on currently saved
patch size.  It is also very easy to stop -- canvas_saveto() of
g_canvas.c is lacking a closing call of

+       binbuf_free(b);
    }

There is also a very small (18 bytes) leak which happens every time
any window is closed.  The two binbufs: t_editor::e_connectbuf and
t_editor::e_deleted should be freed in editor_free() of g_canvas.c:

+   binbuf_free(x->e_connectbuf);
+   binbuf_free(x->e_deleted);
    freebytes((void *)x, sizeof(*x));

Finally there is a small (40-byte) memory amount leaking every time
toplevel patch is closed.  It is due to not freeing memory allocated
to parent `container' object in canvas_new():

    t_subcanvas *y = (t_subcanvas *)pd_new(subcanvas_class);

This is small, infrequent, and probably not worth the fuss, although
there could be some danger of future bugs lurking here.

To stop this leak the canvas destruction in canvas_menuclose():

-   pd_free(&x->x_pd);

should be changed to parent subcanvas destruction:

+   pd_free(&x->x_parent->x_ob.ob_pd);

but this creates another problem -- subcanvas class is patchable,
and pd_free() routine assumes that patchable objects should have
a valid binbuf pointer, while this binbuf is never created for
toplevel parent subcanvases.  There are three options:

1. create the binbuf for toplevel parent subcanvas:

+   if (!super) y->x_ob.ob_binbuf = binbuf_new();

2. use a nonpatchable variant of subcanvas class for toplevel parent;
3. add a check for empty binbuf pointer to pd_free():

+   if (((t_object *)x)->ob_binbuf)
        binbuf_free(((t_object *)x)->ob_binbuf);