-
Notifications
You must be signed in to change notification settings - Fork 996
Add garbage collector that uses an external allocator #991
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
Conversation
fe89d0b to
1a0b36d
Compare
|
This PR should be ready now. |
aykevl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, although I can't say I fully understand it (treaps are foreign to me).
One issue though: this needs some sort of testing. Ideally ./testdata/gc.go on the host, but those integration tests are currently a bit inflexible. Until then I think it would be best to include a smoke test with this GC on the host OS to see whether it compiles at all.
| @@ -0,0 +1,14 @@ | |||
| // +build darwin linux,!baremetal freebsd,!baremetal | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only now spot this freebsd,!baremetal in the code, which doesn't make a lot of sense. The fact that there is linux,!baremetal is because baremetal targets pretend to be linux/arm to get the standard library to compile. Such a !baremetal exclusion is not necessary for FreeBSD.
But this is not something that needs fixing in this PR.
This has actually turned out to be surprisingly effective, and was able to catch tons of bugs.
I am not quite sure what you mean. Don't the existing tests already check that they compile? |
Ah, now I see. You also switched the default GC on Linux etc. Then it's fine. One thing I only now notice: some documentation doesn't include the new GC type, such as the help text in the |
|
Alright, I think I updated all of the docs in this repo. |
|
Thanks for updating! |
In cases where tinygo is running in an environment that it does not own, it may be desirable to allow tinygo to use a C heap. This is accomplished by prepending treap nodes to the beginning of every allocation.
Marking a node is implemented by removing it from the treap and then pushing it onto a scan queue (which is implemented as a linked list using the memory previously used for the treap node). Nodes are then popped off of this queue, scanned, and then pushed onto a new treap. Once the queue is empty, all currently active nodes will have been moved from the old treap to the new treap. All nodes from the old treap are then destroyed, and the new treap is swapped in.
All operations are done non-recursively. This is still somewhat WIP, and there is a lot of debugging code still left in right now.