This is the mail archive of the guile@sourceware.cygnus.com mailing list for the Guile project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: Making Guile slower


Jost Boekemeier <jostobfe@calvados.zrz.TU-Berlin.DE> writes:

> Greg Harvey <Greg.Harvey@thezone.net> writes:
> 
> > Mikael Djurfeldt <mdj@mdj.nada.kth.se> writes:
> > 
> > > Greg Harvey <Greg.Harvey@thezone.net> writes:
> > > 
> > > > Consider this:
> > > > 
> > > > SCM_NEWCELL(foo);
> > > > SCM_SETCDR(foo, scm_must_malloc(foo_size, s_make_foo));
> > > > SCM_SETCAR(foo, scm_tc16_foo);
> > > 
> > > ... which should be written ...
> 
> 
>   m = scm_must_malloc (foo_size, s_make_foo);
>   SCM_NEWCELL (foo);
>   SCM_SETCDR (foo, m);
>   SCM_SETCAR (foo, scm_tc16_foo);
> 
> 
> > Yes, but it rarely is ;)
> 
> 
> There are only 6 places where SCM_NEWCELL is used incorrectly:
> 
> * posix.c (scm_getgroups)
> * strings (scm_makstr)
> * strings (scm_make_shared_substring) 
> * strings (scm_mkstrport)
> * struct.c (scm_make_vtable_vtable)
> * unif.c (scm_make_ra)

You missed the fundamental example (scm_make_vector); I'm sure I could
hunt down more than those that don't do things right; the fact that
you missed cases to show how small a change it is really doesn't fill
me with confidence. What we have is a problem that's easy to screw up
and easy to fix (though the current case doesn't do enough; it should
check for the cdr of an allocated cell for another cell). It is also
adding less to the code than the currently useless increment of
scm_cells_allocated. If anyone wants to be really concerned with
counting every instruction, there's a lot of useless shit in there
that can be trimmed out before we start worrying about changes that
make guile easier to use and less bug-prone.

> You will have the same problem when you create a smob btw.  For example
> the following is wrong:
> ----------------------------------------
> 
>   environment->obarray = 
>     scm_make_vector ((SCM) SCM_MAKINUM (scm_symhash_dim), SCM_EOL);  
>   SCM_NEWCELL (environment_smob);
>   SCM_SETCDR (environment_smob, environment);
>   SCM_SETCAR (environment_smob, scm_tc16_environment);
> 

This isn't the same problem at all; in this case, you are creating a
reference in some place that's most likely unreachable from the gc;
there's nothing to be done about that without conservatively scanning
everything (which would be a big and expensive change).

-- 
Gregh

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]