[PD] abstraction penalty benchmarks

[Miller Puckette]

There still could be situations where an abstraction has a sub-patch ("pd foo"
for instance) - I'm not clear as to whether those namings should be supressed
as well.  It seems like a tricky problem - lots of people seem to use
abstractions with only one instance and might be depending on the bindings.

Maybe the rule should be that before a canvas binds itself to the symbol
it checks to see if anyone else already did and if so silently refuses
(protected with a compatibility flag as usual).

cheers
M

On Fri, Aug 09, 2013 at 03:15:52PM +0100, Claude Heiland-Allen wrote:
> Hi,
> 
> On 09/08/13 04:27, Miller Puckette wrote:
> > Here's a guess - I think each copy of the abstraction binds itself to
> > a symbol, "pd-<name>". Binding is fast bt unbinding is linear-time in the
> > number of things bound to the symbol... ouch.
> 
> Yes, I think that's it:
> 
> git master from yesterday
> Pd-0.45.0 ("test") compiled 17:10:02 Aug  8 2013
> patch   bench   open    dsp     save    close
> small   4.6e-05 38.775  1.34085 44.326  4.012
> medium  5.2e-05 314.812 10.2582 715.956 144.512
> large   4.7e-05 2639.28 85.3561 78711.5 35918
> 
> git master, lightly patched with the attached
> Pd-0.45.0 ("test") compiled 14:36:39 Aug  9 2013
> patch   bench   open    dsp     save    close
> small   4.3e-05 36.167  1.31707 39.657  3.447
> medium  4.4e-05 291.52  9.8958  327.389 18.067
> large   4.3e-05 2443.07 82.0808 3628.66 224.972
> 
> > There's a good reason to bind toplevels and named sub-patches to ther names,
> > but I think there's little reason to do it for abstractions - perhaps I can
> > take this out, but I'd have to leave it as an option for compatibility (ouch!)
> 
> The attached patch doesn't do this, but it should be easy to add a
> global compatibility flag to the new canvas_should_bind() function.
> 
> Toplevel patches are still bound to pd-<name>.pd and subpatches [pd
> <name>] are still bound to pd-<name>; it's just abstraction instances
> that no longer have the automatic binding.
> 
> > On Thu, Aug 08, 2013 at 10:46:24PM -0400, Jonathan Wilkes wrote:
> >> On 08/08/2013 04:57 PM, Claude Heiland-Allen wrote:
> 
> [snip]
> 
> >>> The killer in the bottom right corner: with 8x as many abstractions, it
> >>> takes 100x as long to save an instance, and 200x as long to close the patch.
> 
> [snip]
> 
> >> Any idea why "save" is so much greater than close + open?
> 
> I think Miller had the answer.
> 
> >> Also, any idea what pd is doing for the bulk of that time when
> 
> I haven't profiled yet (I had trouble even getting debug symbols into an
> installed Pd with the autoconf system, no success yet..), so the
> following are just guesses:
> 
> >> saving
> 
> searching for copies of the abstraction to reload, then reloading them
> via some select/cut/undo method (which preserves connections).  also
> unbinding symbols...
> 
> >> closing 
> 
> calling all the destructors. also unbinding symbols...
> 
> >> opening
> 
> refinding/reloading/reparsing/reinstantiating all the abstractions (I
> need to relocate my abstraction cache patches and update them to current Pd)
> 
> >> dsp'ing?
> 
> traversing each canvas to topologically sort its dsp objects and build
> the dsp chain - which gets resizebytes() each time something is added,
> could make it O(log(N)) count of resizes (doubling the size each time)
> but I tried it and it saved about 9 microseconds, not worth the extra
> complexity...
> 
> 
> Claude
> -- 
> http://mathr.co.uk
> 

> From 16fd46b3abe83dba956b78385d01ad19737b6c76 Mon Sep 17 00:00:00 2001
> From: Claude Heiland-Allen <claude at mathr.co.uk>
> Date: Fri, 9 Aug 2013 14:57:14 +0100
> Subject: [PATCH] don't bind abstraction instances to pd-<filename>
> 
> ---
>  src/g_canvas.c |   39 +++++++++++++++++++++++++++++----------
>  1 file changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/src/g_canvas.c b/src/g_canvas.c
> index 804ac57..c7a22d6 100644
> --- a/src/g_canvas.c
> +++ b/src/g_canvas.c
> @@ -55,6 +55,9 @@ void canvas_reflecttitle(t_canvas *x);
>  static void canvas_addtolist(t_canvas *x);
>  static void canvas_takeofflist(t_canvas *x);
>  static void canvas_pop(t_canvas *x, t_floatarg fvis);
> +static int canvas_should_bind(t_canvas *x);
> +static void canvas_bind(t_canvas *x);
> +static void canvas_unbind(t_canvas *x);
>  
>  /* --------- functions to handle the canvas environment ----------- */
>  
> @@ -205,11 +208,9 @@ void canvas_makefilename(t_canvas *x, char *file, char *result, int resultsize)
>  
>  void canvas_rename(t_canvas *x, t_symbol *s, t_symbol *dir)
>  {
> -    if (strcmp(x->gl_name->s_name, "Pd"))
> -        pd_unbind(&x->gl_pd, canvas_makebindsym(x->gl_name));
> +    canvas_unbind(x);
>      x->gl_name = s;
> -    if (strcmp(x->gl_name->s_name, "Pd"))
> -        pd_bind(&x->gl_pd, canvas_makebindsym(x->gl_name));
> +    canvas_bind(x);
>      if (x->gl_havewindow)
>          canvas_reflecttitle(x);
>      if (dir && dir != &s_)
> @@ -379,8 +380,7 @@ t_canvas *canvas_new(void *dummy, t_symbol *sel, int argc, t_atom *argv)
>      x->gl_owner = owner;
>      x->gl_name = (*s->s_name ? s : 
>          (canvas_newfilename ? canvas_newfilename : gensym("Pd")));
> -    if (strcmp(x->gl_name->s_name, "Pd"))
> -        pd_bind(&x->gl_pd, canvas_makebindsym(x->gl_name));
> +    canvas_bind(x);
>      x->gl_loading = 1;
>      x->gl_goprect = 0;      /* no GOP rectangle unless it's turned on later */
>          /* cancel "vis" flag if we're a subpatch of an
> @@ -479,9 +479,8 @@ t_glist *glist_addglist(t_glist *g, t_symbol *sym,
>      x->gl_screeny1 = GLIST_DEFCANVASYLOC;
>      x->gl_screenx2 = 450;
>      x->gl_screeny2 = 300;
> -    if (strcmp(x->gl_name->s_name, "Pd"))
> -        pd_bind(&x->gl_pd, canvas_makebindsym(x->gl_name));
>      x->gl_owner = g;
> +    canvas_bind(x);
>      x->gl_isgraph = 1;
>      x->gl_goprect = 0;
>      x->gl_obj.te_binbuf = binbuf_new();
> @@ -723,8 +722,7 @@ void canvas_free(t_canvas *x)
>          canvas_vis(x, 0);
>      if (x->gl_editor)
>          canvas_destroy_editor(x);   /* bug workaround; should already be gone*/
> -    if (strcmp(x->gl_name->s_name, "Pd"))
> -        pd_unbind(&x->gl_pd, canvas_makebindsym(x->gl_name));
> +    canvas_unbind(x);
>  
>      if (x->gl_env)
>      {
> @@ -985,6 +983,27 @@ int canvas_isabstraction(t_canvas *x)
>      return (x->gl_env != 0);
>  }
>  
> +    /* return true if the "canvas" object should be bound to a name */
> +static int canvas_should_bind(t_canvas *x)
> +{
> +        /* FIXME should have a "backwards compatible" mode */
> +        /* not named "Pd" && (is top level || is subpatch) */
> +    return strcmp(x->gl_name->s_name, "Pd") && (!x->gl_owner || !x->gl_env);
> +}
> +
> +static void canvas_bind(t_canvas *x)
> +{
> +    if (canvas_should_bind(x))
> +        pd_bind(&x->gl_pd, canvas_makebindsym(x->gl_name));
> +}
> +
> +static void canvas_unbind(t_canvas *x)
> +{
> +    if (canvas_should_bind(x))
> +        pd_unbind(&x->gl_pd, canvas_makebindsym(x->gl_name));
> +}
> +
> +
>      /* return true if the "canvas" object should be treated as a text
>      object.  This is true for abstractions but also for "table"s... */
>  /* JMZ: add a flag to gop-abstractions to hide the title */
> -- 
> 1.7.10.4
>