Below is a cleanup patch for a comment or two, some flags missing, that I
did while reading through it all in an attempt to figure it all out.

I was impressed with how straightforward all the code was, but there were
a few things that confused me. If you answer these, I can provide
documentation patches or cleanups to help clarify things that I found
confusing. (And I'm assuming others would find confusing.)

The biggest thing was the interaction and difference between strings and
buffers in the GC system. First, there are three pools in the headers and
the code. Then I saw that buffers and PMCs get better treatment than
strings with their mark_*_unused, trace_active_*,s and free_unused_*. To
top it off, the buffer versions of those functions deal with the STRING
arenas and registers, which seems a bit counterintuitive.

Finally, trace_active_PMCs seems to be the only function to handle
buffers. Looking at the amount of similarity between that and
trace_active_buffers, I think it might help speed to combine those two
functions together, to help avoid the duplicate loops, and to make it a
bit easier to follow.

I think I understand it all now, but I'd like to check to make sure I'm
right. All strings are, and can be treated as, buffers, since they are
referenced by pointer only. The cleanup routines know that it's a string,
and free each to the appropriate pool. Buffers have no high-level access,
and exist only as part of PMCs, so they can only be freed from there.

If my understanding is correct, then what is the purpose of
find_dead_strings and dead_string_run? They both are not implemented and
not called from anywhere. Are they intended to be implemented at some
point, or should they disappear?

ppd09 talks about custom DOD and GC functions, and support for buffers of
buffers, but I'm unable to find code that supports either. Just checking
those are still tbd.

Where does the 'Not yet implemented' come into play for free_unused_PMCs?
It looks implemented and working to me. (Although I did not debug it to
see if it is being called and working.)

Thanks,
Mike Lambert

Index: resources.c
===================================================================
RCS file: /cvs/public/parrot/resources.c,v
retrieving revision 1.33
diff -u -r1.33 resources.c
--- resources.c 22 Mar 2002 20:24:02 -0000      1.33
+++ resources.c 25 Mar 2002 08:11:19 -0000
@@ -76,15 +76,15 @@
   /* Note it in our stats */
   interpreter->total_PMCs += PMC_HEADERS_PER_ALLOC;
   /* Yeah, this is skanky. They're not really active, but
-     add_header_to_free assumes that it's adding an active header to
+     add_pmc_to_free assumes that it's adding an active PMC to
      the free list */
   interpreter->active_PMCs += PMC_HEADERS_PER_ALLOC;

@@ -110,7 +110,7 @@
   /* We return system memory if we've got no interpreter yet */
   if (NULL == interpreter) {
     return_me = mem_sys_allocate(sizeof(PMC));
-    return_me->flags = PMC_live_FLAG;
+    return_me->flags = PMC_live_FLAG | BUFFER_sysmem_FLAG;
     return_me->vtable = NULL;
     return_me->data = NULL;
     return return_me;
@@ -212,7 +212,7 @@
      yet */
   if (interpreter == NULL) {
     return_me = mem_sys_allocate(sizeof(Buffer));
-    return_me->flags = BUFFER_live_FLAG;
+    return_me->flags = BUFFER_live_FLAG | BUFFER_sysmem_FLAG;
     return return_me;
   }

@@ -362,7 +362,7 @@
     struct PRegChunk *cur_chunk;
     /* We have to start somewhere, and the global stash is a good
        place */
-    last = current = interpreter->perl_stash->stash_hash;;
+    last = current = interpreter->perl_stash->stash_hash;
     /* mark it as used and get an updated end of list */
     last = mark_used(current, last);

@@ -617,7 +617,7 @@
      yet */
   if (interpreter == NULL) {
     return_me = mem_sys_allocate(sizeof(STRING));
-    return_me->flags = BUFFER_live_FLAG;
+    return_me->flags = BUFFER_live_FLAG | BUFFER_sysmem_FLAG;
     return return_me;
   }


Reply via email to