Skip to content

Conversation

@niaow
Copy link
Member

@niaow niaow commented Mar 25, 2020

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.

@niaow niaow force-pushed the extalloc branch 4 times, most recently from fe89d0b to 1a0b36d Compare March 27, 2020 16:06
@niaow niaow marked this pull request as ready for review March 27, 2020 16:22
@niaow niaow requested a review from aykevl March 27, 2020 16:22
@niaow
Copy link
Member Author

niaow commented Mar 27, 2020

This PR should be ready now.

Copy link
Member

@aykevl aykevl left a 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
Copy link
Member

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.

@niaow
Copy link
Member Author

niaow commented Mar 28, 2020

Ideally ./testdata/gc.go on the host, but those integration tests are currently a bit inflexible.

This has actually turned out to be surprisingly effective, and was able to catch tons of bugs.

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.

I am not quite sure what you mean. Don't the existing tests already check that they compile?

@aykevl
Copy link
Member

aykevl commented Mar 28, 2020

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.

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 tinygo command (main.go) and the function documentation for (*compileopts.Config).GC.

@niaow
Copy link
Member Author

niaow commented Mar 29, 2020

Alright, I think I updated all of the docs in this repo.

@aykevl aykevl merged commit 62e78c0 into tinygo-org:dev Mar 30, 2020
@aykevl
Copy link
Member

aykevl commented Mar 30, 2020

Thanks for updating!

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