syscall/js, io/fs: fs.Read copies minimally 512 bytes to from JS to Go

[codefromthecrypt]

What version of Go are you using (go version)?

$ go version
go version go1.17.11 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

GOARCH=wasm GOOS=js

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/adrian/Library/Caches/go-build"
GOENV="/Users/adrian/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/adrian/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/adrian/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/adrian/.gimme/versions/go1.17.11.darwin.amd64"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/adrian/.gimme/versions/go1.17.11.darwin.amd64/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.17.11"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/adrian/oss/wazero/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/vd/1cf8zdb1721f4z5rjggy8bp40000gn/T/go-build3326543425=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Compiled a program that ends up using fs.Read and fs.Write.

Ex.

bytes, err := ioutil.ReadFile(os.Args[i])
if err != nil {
	os.Exit(1)
}
os.Stdout.Write(bytes)

What did you expect to see?

I expected the resulting wasm to js.copyBytesToGo no more than the size of the file.

What did you see instead?

js.copyBytesToGo was called with 512 instead because the contents were small and
the nRead is currently ignored in fs_js.go. It is 512 due to special casing upstream for
small files in os.ReadFile.

Here's the snippet of fs.Read

	buf := uint8Array.New(len(b))
	n, err := fsCall("read", fd, buf, 0, len(b), nil)
	if err != nil {
		return 0, err
	}
	js.CopyBytesToGo(b, buf) // < not buf[0:n]

I think this should only copy n bytes as it is more efficient.
It should also skip invoking js.CopyBytesToGo when zero bytes were read.

[codefromthecrypt]

I'm pretty sure this small change will make a lot of impact and happy to implement it unless it is easier for folks already here to do it.

Note this is particularly troublesome as fsCall cannot return EOF due to there being no way to construct it in mapJSError. What this means is that a 17 byte file ends up with two buffer copies.. one 512 to copy the actual 17 bytes and another 495 byte copy (512-17) for the zero byte read mapped heuristically to to EOF due to the lack of EOF support in GOOS=js

cc @bradfitz as I think you've seen some code around this in the past (hint poll.FD.Read)

[dr2chase]

@ianlancetaylor can you give this a look?

[ianlancetaylor]

This is js so @neelance

[neelance]

@codefromthecrypt I have just reproduced your findings. Feel free to implement a fix. I'd be very happy to see you help with Go's support for WebAssembly. 👍

[codefromthecrypt]

ok got around to it #55062

[gopherbot]

Change https://go.dev/cl/430875 mentions this issue: syscall/js: Makes copies from JS to Go more efficient