Re: RFC: Foreign objects facility

[Mark H Weaver <mhw@netris.org>]

Hi Andy,

Andy Wingo <wingo@pobox.com> writes:

> I propose to provide a new interface that will eventually make SMOBs
> obsolete.  This new interface is based on structs with raw fields -- the
> 'u' fields.  (See
> http://www.gnu.org/software/guile/docs/master/guile.html/Vtables.html#Vtables
> for description of 'u' fields.  Note that the documentation is wrong --
> these fields are indeed traced by the GC.)

Sounds like a good idea, in general.

> Here is the proposed C API:
>
>     SCM scm_make_foreign_object_type (SCM name, SCM slot_names,
>                                       scm_t_struct_finalize finalizer);

Shouldn't it be 'scm_t_struct_finalizer', with trailing 'r'?

>     void scm_assert_foreign_object_type (SCM type, SCM val);
>
>     SCM scm_make_foreign_object_1 (SCM type, scm_t_bits val0);
>     SCM scm_make_foreign_object_2 (SCM type, scm_t_bits val0,
>                                     scm_t_bits val1);
>     SCM scm_make_foreign_object_3 (SCM type, scm_t_bits val0,
>                                     scm_t_bits val1, scm_t_bits val2);

If we include an interface like this, I think we should have more of
these, but see below.

>     SCM scm_make_foreign_object_n (SCM type, size_t n, scm_t_bits vals[]);
>
>     scm_t_bits scm_foreign_object_ref (SCM obj, size_t n);
>     void scm_foreign_object_set_x (SCM obj, size_t n, scm_t_bits val);

I'm worried about the type-punning that's implied by this interface.
The C standards are now very strict about this sort of thing, and modern
compilers are becoming increasingly aggressive about taking advantage of
these restrictions to derive assumptions about the code.

For example, having recently researched this, I know that converting
unsigned integers to signed integers is very awkward and potentially
inefficient to do portably, so using 'scm_foreign_object_ref' to extract
a signed integer will be a pain.  It would look something like this
(untested):

--8<---------------cut here---------------start------------->8---
scm_t_signed_bits
scm_foreign_object_signed_ref (SCM obj, size_t n)
{
  scm_t_bits bits = scm_foreign_object_ref (obj, n);

/* GNU C specifies that when casting to a signed integer of width N, the
   value is reduced modulo 2^N to be within range of the type.  Neither
   C99 nor C11 make any guarantees about casting an out-of-range value
   to a signed integer type.  */

#ifndef __GNUC__
  if (bits > SCM_T_SIGNED_BITS_MAX)
    return -1 - (scm_t_signed_bits) ~bits;
#endif
  return (scm_t_signed_bits) bits;
}
--8<---------------cut here---------------end--------------->8---

Portability is more problematic for pointer types.  The C standards make
no guarantees about the semantics of converting between pointers and
integers, and it's not clear to me how future proof this will be.  One
related issue is that I'd like to leave open the possibility that
'scm_t_bits' might be larger than a pointer.

For slot refs and sets, I suggest that we need to have multiple variants
for different types.  At the very least, we'll need variants for signed
integers, unsigned integers, and pointers.

For constructors, I think we should provide a macro that handles the
conversion from pointer to scm_t_bits.  (Converting from signed to
unsigned is portable, so there's no issue there).

> The overhead of a foreign object is two words -- the same as the
> overhead on any struct.  (Compare to SMOBs, which have a half-word
> overhead.)

It would be good to think about how we might someday reduce this
overhead in the future, and to make sure we don't make decisions that
would prevent us from doing that.

> From a12efcfaae1c65cc703616ea15106a88efba3f55 Mon Sep 17 00:00:00 2001
> From: Andy Wingo <wingo@pobox.com>
> Date: Sun, 27 Apr 2014 14:47:40 +0200
> Subject: [PATCH] New foreign object facility, to replace SMOBs
>
> * libguile/foreign-object.c:
> * libguile/foreign-object.h:
> * module/system/foreign-object.scm:
> * test-suite/standalone/test-foreign-object-c.c:
> * test-suite/standalone/test-foreign-object-scm: New files.
> * test-suite/standalone/Makefile.am:

Something should be added to the line above.

> diff --git a/libguile/foreign-object.c b/libguile/foreign-object.c
> new file mode 100644
> index 0000000..78b017a
> --- /dev/null
> +++ b/libguile/foreign-object.c
> @@ -0,0 +1,187 @@
> +/* Copyright (C) 2014 Free Software Foundation, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public License
> + * as published by the Free Software Foundation; either version 3 of
> + * the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> + * 02110-1301 USA
> + */
> +
> +
> +\f
> +#ifdef HAVE_CONFIG_H
> +#  include <config.h>
> +#endif
> +
> +#include "libguile/_scm.h"
> +#include "libguile/goops.h"
> +#include "libguile/foreign-object.h"
> +
> +
> +\f
> +
> +static SCM make_fobj_type_var;
> +
> +static void
> +init_make_fobj_type_var (void)
> +{
> +  make_fobj_type_var = scm_c_private_lookup ("system foreign-object",
> +                                             "make-foreign-object-type");
> +}
> +
> +SCM
> +scm_make_foreign_object_type (SCM name, SCM slot_names,
> +                              scm_t_struct_finalize finalizer)
> +{
> +  SCM type;
> +
> +  static scm_i_pthread_once_t once = SCM_I_PTHREAD_ONCE_INIT;
> +  scm_i_pthread_once (&once, init_make_fobj_type_var);

Good!

> +
> +  type = scm_call_2 (scm_variable_ref (make_fobj_type_var), name, slot_names);
> +
> +  if (finalizer)
> +    SCM_SET_VTABLE_INSTANCE_FINALIZER (type, finalizer);
> +
> +  return type;
> +}
> +
> +void
> +scm_assert_foreign_object_type (SCM type, SCM val)
> +{
> +  if (!SCM_IS_A_P (val, type))
> +    scm_error (scm_arg_type_key, NULL, "Wrong type (expecting ~A): ~S",
> +               scm_list_2 (scm_class_name (type), val), scm_list_1 (val));
> +}
> +
> +SCM
> +scm_make_foreign_object_1 (SCM type, scm_t_bits val0)
> +{
> +  return scm_make_foreign_object_n (type, 1, &val0);
> +}
> +
> +SCM
> +scm_make_foreign_object_2 (SCM type, scm_t_bits val0, scm_t_bits val1)
> +{
> +  scm_t_bits vals[2] = { val0, val1 };

This non-constant initializer depends on C99.  We discussed this on IRC,
and IIRC we agreed that we should transition to requiring C99 for 2.2,
but making that change in 2.0 is potentially disruptive.

> +
> +  return scm_make_foreign_object_n (type, 2, vals);
> +}
> +
> +SCM
> +scm_make_foreign_object_3 (SCM type, scm_t_bits val0, scm_t_bits val1,
> +                           scm_t_bits val2)
> +{
> +  scm_t_bits vals[3] = { val0, val1, val2 };

Ditto.

> +
> +  return scm_make_foreign_object_n (type, 3, vals);
> +}
> +
> +SCM
> +scm_make_foreign_object_n (SCM type, size_t n, scm_t_bits vals[])
> +#define FUNC_NAME "make-foreign-object"
> +{
> +  SCM obj;
> +  SCM layout;
> +  size_t i;
> +
> +  SCM_VALIDATE_VTABLE (SCM_ARG1, type);
> +
> +  layout = SCM_VTABLE_LAYOUT (type);
> +
> +  if (scm_i_symbol_length (layout) / 2 < n)
> +    scm_out_of_range (FUNC_NAME, scm_from_size_t (n));
> +
> +  for (i = 0; i < n; i++)
> +    if (scm_i_symbol_ref (layout, i * 2) != 'u')
> +      scm_wrong_type_arg_msg (FUNC_NAME, 0, layout, "'u' field");

This looks inefficient.  How about using 'scm_i_symbol_chars'?

> +
> +  obj = scm_c_make_structv (type, 0, 0, NULL);
> +
> +  for (i = 0; i < n; i++)
> +    SCM_STRUCT_DATA_SET (obj, i, vals[i]);
> +
> +  return obj;
> +}
> +#undef FUNC_NAME
[...]

    Thanks!
      Mark