Skip to content

Unneccesary alloc in hot path #2237

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
oflebbe opened this issue Nov 6, 2021 · 5 comments
Closed

Unneccesary alloc in hot path #2237

oflebbe opened this issue Nov 6, 2021 · 5 comments
Assignees

Comments

@oflebbe
Copy link
Contributor

oflebbe commented Nov 6, 2021

I noticed that there are IMO unnecessary allocations generated by the compiler in the hot path if one uses the st7789 SPI driver on rp pico .
Looks like it might slow down SPI transmission considerably.

In order to debug it, I stripped down the code to the bare minimum still showing the diagnostics.
Please use tinygo (from dev branch) and compile

tinygo build -o a    -print-allocs=. main.go
...main.go:14:14: object allocated on the heap: escapes at line 14

If you comment the switch or the print statement this warning goes away.
main.txt

@oflebbe
Copy link
Contributor Author

oflebbe commented Nov 6, 2021

For easier read here the source code inline

package main

type MockSPI int

func (m MockSPI) Tx(w, r []byte) (err error) {
	switch {
	case w == nil:
		print(1)
	}
	return err
}

func (d MockSPI) SetWindow() {
	d.Tx([]uint8{1}, nil)
}

func main() {
	var m MockSPI = 1
	m.SetWindow()
}

@oflebbe oflebbe changed the title Unneccesary alloc in Hot path Unneccesary alloc in hot path Nov 6, 2021
@dgryski
Copy link
Member

dgryski commented Oct 21, 2024

So it's just the slice header here. If we replace the slice with a pointer to an array then we're ok.

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

type MockSPI int

func (m MockSPI) Tx(w *[1]byte, r []byte) (err error) {
	switch {
	case w == nil:
		print(1)
	}
	return err
}

func (d MockSPI) SetWindow() {
	d.Tx(&[...]byte{1}, nil)
}

func main() {
	var m MockSPI = 1
	m.SetWindow()
}

This seems like the sort of thing we ought to be able to fix.

@dgryski
Copy link
Member

dgryski commented Oct 21, 2024

This is what the Go SSA looks like for SetWindow

func (d MockSPI) SetWindow():
0:                                                                entry P:0 S:0
	t0 = new [1]byte (slicelit)                                    *[1]byte
	t1 = &t0[0:int]                                                   *byte
	*t1 = 1:byte
	t2 = slice t0[:]                                                 []byte
	t3 = (MockSPI).Tx(d, t2, nil:[]byte)                              error
	return

@aykevl
Copy link
Member

aykevl commented Oct 22, 2024

I've fixed this specific issue a while ago in the driver: tinygo-org/drivers#545

@deadprogram
Copy link
Member

Closing since was addressed. Thanks everyone!

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

No branches or pull requests

4 participants