tinygo icon indicating copy to clipboard operation
tinygo copied to clipboard

Optimize `[]byte` to `string` conversions

Open lpereira opened this issue 1 year ago • 1 comments

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

lpereira avatar Jun 08 '24 02:06 lpereira

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.

lpereira avatar Jun 11 '24 17:06 lpereira

Rebased to current dev and marked it as ready for review.

lpereira avatar Jul 23 '24 22:07 lpereira

Ping @aykevl

dgryski avatar Aug 06 '24 01:08 dgryski

I did a quick check and this doesn't seem to optimize the bytes.Equal case?

aykevl avatar Aug 08 '24 12:08 aykevl

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]
}

aykevl avatar Aug 08 '24 12:08 aykevl

(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 avatar Aug 08 '24 23:08 lpereira

@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.

aykevl avatar Aug 09 '24 08:08 aykevl