tinygo
tinygo copied to clipboard
Optimize `[]byte` to `string` conversions
This enables the optimization of functions such as "bytes.Equal()", which is currently implemented as:
func Equal(a, b []byte) bool { return string(a) == string(b) }
Which causes allocations and copies in TinyGo, but not in Big Go.
Since we can prove that neither conversions will escape in this case, we can convert the calls to runtime.stringFromBytes() to an inlined version of it that doesn't allocate or copies the original string.
This is a draft PR, because, although I can verify that the relevant calls to the runtime alloc calls are removed from the final binary in a simple test program, make test is failing. I'd like some help here to understand what's happening.
Another thing that I didn't have time today to check was why isReadOnly() was returning false for my test program; I'll pick this up on Monday to take a closer look, but for now, I'm omitting the call here, which is most likely why make test isn't happy.
Closes: #4045 CC: @aykevl @dgryski
Rebased and pushed a new version, that also improves the read-only checks to catch more usages. I still need to fine-comb the table I added for this; however, local tests (make tinygo-test-wasi) seem to be working, but I don't have all the dependencies for things other than WASM building, so I'll wait for the CI to see if I need to fix something unexpected.
Rebased to current dev and marked it as ready for review.
Ping @aykevl
I did a quick check and this doesn't seem to optimize the bytes.Equal case?
This optimization pass should have at least one test that shows that it actually does what it says that it does, and maybe a few tests that show that it doesn't apply in incorrect cases.
I think this is actually pretty tricky to get right. For example, in the following case the optimization would be legal:
func foo(b []byte) {
s := string(b)
return s[0]
}
but in the following case, it would not be legal:
func foo(b []byte) {
s := string(b)
bar() // might modify b
return s[0]
}
(I'm sorry, but I won't be pursuing this optimization anymore as this was done as part of my previous job. If someone wants to pick this up, please feel free to.)
@lpereira thanks for the update!
That's unfortunate, but understandable. I'll close the PR to avoid clutter, but of course it can be reopened if you ever decided to continue work on it.