Marcus Crestani wrote:
>This patch introduces an incremental garbage collector for XEmacs.
>All changes to the code are conditioned on `NEW_GC'. To enable the
>new garbage collector, configure with `--enable-newgc'.
>
>
Marcus, this is awesome! thanks very much for all your work. i
especially appreciate the fact that you've done the work to get this
working on windows (and mac) as well as unix, provided appropriate
versions of gdbinit and such, and taken care of a lot of other little
details that really need to be done in order for this to become really
usable rather than just an "80%" research project tnat never's quite
ready for prime time. you also clearly understand the XEmacs code well
and have obviously been very careful in the way you've written and
tested your code.
i'll take a look at this more extensively later, but a few comments:
-- i think we should go ahead and remove the conditionalization on the
parts of your patch that convert internal structures to lisp objects.
it won't hurt to have the structures be lisp objects always, even with
the old gc, and it will make the code much simpler, as well as easier to
maintain
-- my biggest concern is with gc.c. you just copied a lot of stuff from
alloc.c, and this tends to lead to bit-rot. is there a way you could
conditionalize it so the functions that you need more or less unchanged
remain in alloc.c or are split out into some other file? go ahead and
put new code in gc.c, but duplicated code is bad bad bad as it leads to
bit-rot and maintenance headaches.
-- once you commit this code, i'll start working on stuff you missed,
like in file-coding.c/mule-coding.c, where you have non-Lisp struct
coding_stream, which contains a DATA pointer to a separate set of
instance data, one per type of coding system, which is also a non-Lisp
structure often with Lisp elements in it. I'd suggest reworking this to
follow the lead of the main coding-system and specifier objects, and
incorporate extra data as part of the object itself, making the object
stretchy. that reduces greatly the need to create new objects for each
new subtype, and simplifies creation of new subtypes. (I'd suggest doing
the same for frame, device, etc. for similar reasons, although that can
wait.)
-- since you are creating a lot of new internal objects, i'd suggest a
minor change to the semantics of the object print method. currently if
you set it to 0, if calls default_object_printer. for internal objects,
which should never escape to Lisp level, we want internal_object_printer
instead. currently, even looking among the modules, there is only one
object (toolbar-button) that relies on the current behavior by
specifying 0 to get default_object_printer, while a number of others
(the event-*-data objects -- granted, not currently used) also specify 0
but really want internal_object_printer. your new internal objects do
the same. so i'd suggest just changing things in print_internal() to
call internal_object_printer when there is no print method, and change
the print method for toolbar-button to default_object_printer.
overall, thank you very, very much for this work and for your continued
commitment to work on generational algorithms and other state-of-the-art
stuff!
ben