Skip to content

runtime timers are unimplemented #1037

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

Closed
ryskiwt opened this issue Apr 8, 2020 · 16 comments
Closed

runtime timers are unimplemented #1037

ryskiwt opened this issue Apr 8, 2020 · 16 comments
Labels
enhancement New feature or request

Comments

@ryskiwt
Copy link

ryskiwt commented Apr 8, 2020

Using time.NewTicker causes a runtime error:

package main

import (
	"time"
)

func main() {
	ticker := time.NewTicker(time.Second)
	defer ticker.Stop()

	<-ticker.C

	neverDone := make(chan struct{})
	<-neverDone
}

Error:

Uncaught (in promise) LinkError: WebAssembly.instantiate(): Import #12 module="env" function="time.startTimer" error: function import requires a callable

Compiled using the 0.12.0 docker image:

docker run ... \
    -e GOOS=js -e GOARCH=wasm \
    tinygo/tinygo:0.12.0 tinygo build -target wasm -o hoge.wasm .
@niaow niaow changed the title time.NewTicker / time.NewTimer causes a runtime error: runtime timers are unimplemented Apr 8, 2020
@niaow
Copy link
Member

niaow commented Apr 8, 2020

TinyGo does not implement the runtimeTimer system which is used to implement time.NewTicker and time.AfterFunc. We either need to somehow implement it (not currently possible due to how //go:linkname is implemented), or replace parts of the time package.

@ryskiwt
Copy link
Author

ryskiwt commented Apr 8, 2020

Thanks for reply. I got it.
Is there any timeline or roadmap for implementing go:linkname ?

@niaow
Copy link
Member

niaow commented Apr 8, 2020

So //go:linkname has been implemented, but in TinyGo it preserves type information. Since time.runtimeTimer is an unexported type, another package cannot linkname time.startTimer, because the types would be different and cause a compiler error.

@niaow
Copy link
Member

niaow commented Apr 8, 2020

For now, if possible, use time.Sleep or loops. I do not know when we are going to be able to efficiently implement this.

@deadprogram deadprogram added the enhancement New feature or request label Apr 12, 2020
@niaow
Copy link
Member

niaow commented Apr 26, 2020

As a workaround, I wonder whether we could do something like //go:linkname but with types.

@aykevl
Copy link
Member

aykevl commented Apr 29, 2020

Maybe we need to support //go:linkname with slightly-mismatched types after all. Yes, we could allow custom type names for named types, but doing something like that would still be quite invasive. I still don't like the //go:extern trick for globals, it requires parsing the AST. I would prefer to avoid one more dependency on having the AST available while building the LLVM IR.

Slightly-mismatched types would be much less of a problem if opaque pointer types were completed in LLVM. Unfortunately, it looks like it'll still take quite a while (if it is ever finished). Until then, maybe the simplest solution is the following: if a function to call has a changed linkname (with //go:linkname), then check whether the types are the same and if not bitcast the function pointer. I think that would be the least invasive change to the compiler. Perhaps it would be easier after #1008.

@deadprogram deadprogram added the wasm WebAssembly label May 29, 2020
@deadprogram
Copy link
Member

I was just doing some experimenting with trying to run https://github.com/mbertschler/dragon-iss-docking-autopilot using TinyGo and ran into this exact problem pretty quickly.

Was reading the proposed work-arounds. Any more ideas on this currently, or do we just need to get #1008 merged in and proceed from there?

@aykevl
Copy link
Member

aykevl commented May 29, 2020

#1008 is not really related, it would just make one implementation a bit easier.

I had another idea how //go:linkname could be fixed: every pointer to a named struct could be cast to *i8 or similar during the call. I don't expect that to affect optimizations, but it should be checked to be sure. It does add a bit of noise to the IR, however. And it might make some TinyGo optimization passes more complicated.

@niaow niaow removed the wasm WebAssembly label Jul 6, 2020
@deadprogram
Copy link
Member

I still really would like to be able to write code like

case <-time.NewTimer(3 * time.Second).C:

@aykevl
Copy link
Member

aykevl commented Sep 13, 2020

Yes, and we will need this to support the stdlib testing package (which makes use of these timers).

@aykevl
Copy link
Member

aykevl commented Sep 21, 2020

I have an implementation for timers and tickers, will clean up the patch and send a PR soon.

@aykevl
Copy link
Member

aykevl commented Sep 22, 2020

Here is the PR: #1402

@Zyko0
Copy link

Zyko0 commented Nov 11, 2021

Any update on this ?

@aykevl
Copy link
Member

aykevl commented Nov 11, 2021

I think I can take a look after #2011 has been merged. The current PR is way out of date.

@Zyko0
Copy link

Zyko0 commented Nov 21, 2021

Any idea about when this could happen now #2011 has been merged ?
Thanks !

@deadprogram deadprogram removed the next-release Will be part of next release label Sep 30, 2022
@deadprogram
Copy link
Member

Released as part of v0.26.0

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

No branches or pull requests

5 participants