Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Garbage collector reimplementation #25

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

garrisonhh
Copy link
Contributor

A pretty simple mark-sweep garbage collector. In most cases this should be similarly performant to a bump allocator.

The most interesting thing about it is the used/free object tracking, which uses a bit set in with a binary-heap indexing strategy to keep track of objects that are either in use or not collected yet, the idea being that looking for a free object in a relatively full list should remain fast. I'm not sure this is actually more useful than keeping a simple list of freed indices, but I thought the idea was worth taking a stab at.

src/Gc.zig Show resolved Hide resolved
src/Gc.zig Show resolved Hide resolved
/// iterate the generation, collect within each page
fn collect(gc: *Gc) void {
gc.generation +%= 1;
gc.markVal(gc.base_frame);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous root marking system also detected temporary values on the program's stack. Only marking values referenced by the virtual machine's stack leaves those values susceptible to being collected while in use.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, ok. How can I access those values during marking?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what the @frameAddress stuff was for. It wasn't perfect by any means and seemingly didn't work on windows but I never got around to debugging it further.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have two ideas for a solution to this:

  1. Reference frames on the caller's stack during the callee's runtime
  2. Tracking the current call stack in the VM

I think I like the first better since it requires very few any additional mechanics to be added. These also would be reusable for creating error traces, or do error traces already exist? What do you think of these solutions?

@@ -280,25 +274,23 @@ pub fn compileAndRun(vm: *Vm, file_path: []const u8) !*Value {
else => |e| return e,
};

var frame = Frame{
const frame = try vm.gc.gpa.create(Frame);
frame.* = Frame{
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the defer here leaks the stack and err_handlers and now that the frame is allocated it also needs to be freed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the source of a segfault that I spent an hour or so debugging. Since my implementation tracks the base frame like any other value, it gets collected after clearBaseFrame() is called and this shouldn't leak anything

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it isn't stored on the heap, there has to be a special case added to either Value.deinit or gc.collect or both and that felt like a bit of a dirty solution

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Freeing the garbage collector pages doesn't free the array lists allocated with the gpa. I previously reset the frame value to an int to avoid the segfault:

defer frame_val.* = .{ .int = 0 }; // clear frame

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which arraylists are you referring to? I feel that it would generally be preferable to be able to treat frames the same as any other value, or is there something I'm missing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants