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

tinygo support #23

Merged
merged 1 commit into from
Jul 22, 2021
Merged

tinygo support #23

merged 1 commit into from
Jul 22, 2021

Conversation

bradleypeabody
Copy link
Contributor

@bradleypeabody bradleypeabody commented Jul 22, 2021

This PR makes fwd compile and test correctly with TinyGo. After some review, I think this change makes more sense than using the not-unsafe version under TinyGo - since unnecessary allocations are particularly problematic in microcontrollers environments.

The actual change is very minor: just one cast from int to uintptr. But I had to copy the function and make a TinyGo-specific version.

For reasons explained here, reflect.SliceHeader is defined slightly differently in TinyGo vs Go.

The tests now pass in TinyGo with:

GO111MODULE=off tinygo test .

Or if you have Docker installed but not TinyGo, you can do:

docker run --rm -v $(pwd):/src -e GO111MODULE=off tinygo/tinygo:latest bash -c 'cd /src/ && tinygo test .'

@bradleypeabody
Copy link
Contributor Author

Also just FYI, I can think of a few other microcontroller-specific changes that should probably be done. E.g. the default sizes should probably be lowered for TinyGo (I'm thinking more like 64 or maybe 256 bytes instead of 2k). And allowing the caller to explicitly provide a read or write buffer, so they could do things like use an array allocated at package level which TinyGo will put in the BSS instead of doing a heap allocation (helps avoid memory fragmentation).

But rather than getting into all of that here in this PR, I think it will be most productive to just start with this PR and then perform whatever TinyGo-specific tuning as part of the changes to msgp and then wrap back around and do a separate PR for fwd if needed.

@philhofer philhofer merged commit 5c56ac6 into philhofer:master Jul 22, 2021
@philhofer
Copy link
Owner

Thanks

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