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

Enable wasm pointer tracking for gc=none. #3280

Merged
merged 1 commit into from
Nov 15, 2022

Conversation

anuraaga
Copy link
Contributor

@anuraaga anuraaga commented Nov 9, 2022

Fixes #3278

This takes the simpler approach of allowing wasm pointer tracking on the stack to be enabled even with gc=none. none is documented as being useful for combining with a separate garbage collector. However in practice, it is basically impossible to do this with wasm because tracking pointers on the stack is required for any GC to be able to see them to mark them.

It seems reasonable to enable the stack tracking code even when not using the TinyGo GC. Either a) an external GC is being used and the tracking is required or b) an external GC is not being used meaning there are no allocations, and therefore no tracking will actually happen.

Having the ability to swap in a custom GC allows TinyGo compiled wasm to perform quite well with low pause times, so this is very enabling for projects like coraza-proxy-wasm

Copy link
Member

@dgryski dgryski left a comment

Choose a reason for hiding this comment

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

LGTM. Seems safe enough.

@deadprogram
Copy link
Member

Thank you for the addition @anuraaga and to @dgryski for reviewing. Now merging.

@deadprogram deadprogram merged commit 0b3a728 into tinygo-org:dev Nov 15, 2022
@aykevl
Copy link
Member

aykevl commented Nov 17, 2022

I don't think we should do this, to be honest. There are many things needed to integrate a GC, and -gc=none is not really able to capture it all. The -gc=none flag was mostly intended for debugging and to make sure binaries have the smallest possible code size. Enabling pointer tracking defeats this purpose.

@deadprogram
Copy link
Member

Hmm, perhaps I merged too quickly then. 😿

@aykevl
Copy link
Member

aykevl commented Nov 17, 2022

On the other hand, while I'm not entirely sold on -gc=custom yet, if we are going to support custom GCs then using -gc=custom is in my opinion the correct way to do so.

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.

4 participants