Ticket #980 (closed patch: done)

Opened 12 years ago

Last modified 12 years ago

GC Refactor

Reported by: jrtayloriv Owned by:
Priority: normal Milestone:
Component: GC Version: 1.5.0
Severity: medium Keywords:
Cc: Language:
Patch status: new Platform: all


Attached is a patch which does some minor refactoring of the GC system. Most of my changes are in an attempt to get closer to being able to choose our GC system with a command line switch, instead of having to choose it at compile time.

This is a very rough patch, but I'd like to get some feedback on the concepts behind it to see if you all think the changes are appropriate, and then perhaps create a branch to do a lot more work on it. Currently it is passing all tests on x86-64 Linux.

For starters, I have done the following:

1) I have gotten rid of all of the #if directives (PARROT_GC_MS, PARROT_GC_GMS, etc) related to selecting the GC system. I have moved this stuff into system-specific header files (i.e. src/gc/generational_ms.h), or wherever was appropriate. I have defined a macro PARROT_GC_DEFAULT_SYS (currently set to 'MS') in include/parrot/settings.h, which will be used to represent the default GC system (i.e. the one that's used if no --gc command line switch is passed).

2) I added a GC_subsystem struct * gc_sys to the Parrot_Interp struct. This structure will contain all data structures and functions (via function pointers) related specifically to the current GC subsystem we are using.

3) I have also started to add something similar to GC_Subsytem to the Small_Object_Pool structure, which was having adding fields added based on preprocessor directives. At the moment, is is just a union (gc_sys_private_data) which contains special data structures related to the Small_Object_Pool for the GC subsystems that need them, but ultimately, I'm going to make it look like the gc_sys struct (where it also contains function hooks related to pools such as add_free_object, etc). And I will do the same for Arenas....

4) Started converting macros like GC_WRITE_BARRIER into proper functions. These functions will be registered as hooks by assigning functions to the function pointers stored in the gc_sys structure. A GC subsystem can register to use these hooks only if they are needed, relieving us of the need to use preprocessor directives to make this decision.

5) On recommendation from WhiteKnight++ I also created API functions Parrot_gc_write_barrier() and Parrot_gc_write_barrier_key() in src/api.c to access the write_barrier hooks in interp->gc_sys, so that nobody has to go poking around in the internals of the interp structure to use them.

Some things I want to do in the (very near) future are:

1) do further cleanup on the Arenas and Small_Object_Pool structures, and many other things

2) Move more system-specific functions out of places like api.c and and into the gc_sys structure

3) Create a command line switch --gc or --gc_sys or whatever, and set it up to

4) Try to find a way to untangle GMS from the entire Parrot internals ... and begin designing a new GMS system from scratch that is better encapsulated.

Anyhow, I am not a very strong C programmer, so there were a view areas where I ran into problems, and just fudged to get things working. A more experienced programmer could probably immediately figure out what to do, in these cases. Namely:

1) My PObj_to_Arena/Arena_to_PObj hooks in gc_sys are not correct (these don't really matter right now, since they're GMS only anyway though)

2) I've left tons of broken stuff related to GMS commented out. These things were broken before I did anything to them, but now we actually have to deal with them since they're not in preprocessor statements anymore, and I probably broke them a bit more :)

3) I didn't know how I should die if the switch statement in src/gc/api::Parrot_gc_initialize() reaches it's default case.

4) I wasn't sure exactly how to tell headerizer about the type signatures of Parrot_gc_write_barrier() and Parrot_gc_write_barrier_key() in src/api.c

I welcome any comments/suggestions on this.

Attached is the 'svn diff' patch, and the extra file that I created (src/gc/generational_ms.h). (Sorry, couldn't figure out how to make 'svn diff' output the contents of the new file as well)



generational_ms.h Download (2.9 KB) - added by jrtayloriv 12 years ago.
gc-refactor.patch Download (33.0 KB) - added by jrtayloriv 12 years ago.
remove weird gc.t thing, and move important stuff to top for readability

Change History

Changed 12 years ago by jrtayloriv


Changed 12 years ago by jrtayloriv

remove weird gc.t thing, and move important stuff to top for readability

Changed 12 years ago by dukeleto

There is now a branch, gc-refactor, for these patches.

Changed 12 years ago by jkeenan

  • priority changed from minor to normal
  • severity changed from low to medium
  • summary changed from [PATCH]: GC Refactor to GC Refactor

Changed 12 years ago by bacek

  • status changed from new to closed
  • resolution set to done

Branch gc-refactor was merged into trunk some time ago. Closing ticket.

Note: See TracTickets for help on using tickets.