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

Add support for using wizer to handle running initAll() #3655

Draft
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

dgryski
Copy link
Member

@dgryski dgryski commented Apr 14, 2023

The largest slow down for compiling programs frequently comes from running interp on initAll(), espcially if there are global variables with complex initializations, such as regular expression or other expensive function calls. This PR allows that phase to be run by wizer instead. This speeds up compilation of a large program from 20+ minutes down to about 2 (including the runtime for wizer).

@dgryski
Copy link
Member Author

dgryski commented Apr 14, 2023

The panic added to syscall is to catch places that might keep the cached libc environment during startup. Otherwise bugs like this would be very hard to track down.

Sadly, the functions added to runtime.initAll() don't appear to have debug information associated with them.

panic: syscall.Environ() called during runtime preinitialization
Error: the `runtime.wizerInit` function trapped

Caused by:
    wasm trap: wasm `unreachable` instruction executed
    wasm backtrace:
        0: 0x4538 - runtime.abort
                        at /Users/dgryski/go/src/github.com/tinygo-org/tinygo/src/runtime/runtime_tinygowasm.go:70:6
                  - runtime._panic
                        at /Users/dgryski/go/src/github.com/tinygo-org/tinygo/src/runtime/panic.go:52:7
        1: 0x1229a - syscall.Environ
                        at /Users/dgryski/go/src/github.com/tinygo-org/tinygo/src/syscall/syscall_libc.go:251:8
                  - os.Environ
                        at /Users/dgryski/go/src/github.com/tinygo-org/tinygo/src/os/env.go:140:24
        2: 0x11918 - <unknown>!runtime.initAll
        3: 0xe2ee - runtime.wizerInit
                        at /Users/dgryski/go/src/github.com/tinygo-org/tinygo/src/runtime/runtime_wasm_wasi.go:35:9

error: wizer failed: exit status 1

So figuring out exactly who called os.Environ() is still hard. But at least we can detect it happened.

@gedw99
Copy link

gedw99 commented Apr 15, 2023

Adding myself to this so i can track this and try it out...

@dgryski
Copy link
Member Author

dgryski commented Apr 19, 2023

Something about wizer causes encoding/json to fail. Need to hunt down the exact circumstances.

@dgryski dgryski marked this pull request as draft April 19, 2023 14:25
@dgryski
Copy link
Member Author

dgryski commented Apr 19, 2023

Thought: without the interp pass, certain variables don't know have concrete types and a later pass in transform (interface-lowering?) can't make a decision and defers to runtime, but that's what blows up.

~/go/src/github.com/dgryski/bug/wiz $ cat main.go
package main

import (
	"encoding/json"
)

type Config struct {
	User string
}

const blob = `{ "user": "dgryski" }`

func readConfig() Config {
	var conf Config
	println("hello, main.init")
	if err := json.Unmarshal([]byte(blob), &conf); err != nil {
		panic(err)
	}
	return conf
}

func main() {
	config := readConfig()
	println("user is", config.User)
}
~/go/src/github.com/dgryski/bug/wiz $ tinygo run -target=wasi main.go
hello, main.init
user is dgryski
~/go/src/github.com/dgryski/bug/wiz $ tinygo run -target=wasi -wizer-init main.go
hello, main.init
panic: reflect: unimplemented: AssignableTo with interface json.Marshaler and *string
Error: failed to run main module `/var/folders/5b/_hr1d1fd3qn4t9vtfx5p61wm0000gp/T/tinygo532954112/main`

Caused by:
    0: failed to invoke command default
    1: error while executing at wasm backtrace:
           0: 0x470b - runtime.abort
                           at /Users/dgryski/go/src/github.com/tinygo-org/tinygo/src/runtime/runtime_tinygowasm.go:70:6              - runtime._panic
                           at /Users/dgryski/go/src/github.com/tinygo-org/tinygo/src/runtime/panic.go:52:7
           1: 0x280e - (*reflect.rawType).AssignableTo
                           at /Users/dgryski/go/src/github.com/tinygo-org/tinygo/src/reflect/type.go:932:8
           2: 0xed68 - (*reflect.rawType).Implements
                           at /Users/dgryski/go/src/github.com/tinygo-org/tinygo/src/reflect/type.go:941:23              - (Go interface method)
                           at <Go interface method>
           3: 0x49c19 - encoding/json.newTypeEncoder
                           at /Users/dgryski/go/src/go.googlesource.com/go/src/encoding/json/encode.go:420:80

@gedw99
Copy link

gedw99 commented Apr 20, 2023

oh does

Something about wizer causes encoding/json to fail. Need to hunt down the exact circumstances.

ah encoding/json works on tinygo now ? I know xml does...

@aykevl
Copy link
Member

aykevl commented May 4, 2023

Maybe I already said this elsewhere, but I'm pretty sure that this is OptimizeReflectImplements. Normally it is able to resolve the Implements calls at compile time but not anymore without the global interp pass.

Not sure how to fix that, without actually implementing Implements.

@dgryski
Copy link
Member Author

dgryski commented May 4, 2023

Yes, I believe this wizer issue and the pointer tagging one are similar. In both cases, the compiler is unable to figure out the proper type. In one it's because it's a "fake" **T type that doesn't have a global associated with it, in the other the associated variables haven't been created because the interp pass hasn't run so code is still in IR form rather than resolved values.

@dgryski
Copy link
Member Author

dgryski commented Jul 26, 2023

Rebased this onto dev and my failing test program now works. (Maybe due to some fixes that landed in #3737 ?)

@dgryski
Copy link
Member Author

dgryski commented Jun 4, 2024

I've rebased this branch onto dev and my test is failing again. I wonder if either it wasn't fixed and I didn't test it properly, or it was fixed but additional changes since then broke it again. I'll start looking at OptimizeReflectImplements and see if there's anything we can do to move this forward.

@dgryski
Copy link
Member Author

dgryski commented Aug 22, 2024

Merging #4375 should solve the problem with reflect.

@dgryski
Copy link
Member Author

dgryski commented Aug 22, 2024

We still have the problem of running init() functions in goroutines, but wizer and the scheduler won't play nicely together.

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.

3 participants