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

net: useless memory allocation in writeBuffers on Windows #19222

Closed
MichaelMonashev opened this issue Feb 21, 2017 · 8 comments

Comments

Projects
None yet
4 participants
@MichaelMonashev
Copy link

commented Feb 21, 2017

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

go version go1.8 windows/386

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

set GOARCH=386
set GOBIN=
set GOEXE=.exe
set GOHOSTARCH=386
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Temp\Go
set GORACE=
set GOROOT=C:\Go
set GOTOOLDIR=C:\Go\pkg\tool\windows_386
set GCCGO=gccgo
set GO386=
set CC=gcc
set GOGCCFLAGS=-m32 -mthreads -fmessage-length=0
set CXX=g++
set CGO_ENABLED=1
set PKG_CONFIG=pkg-config
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2

What did you do?

Call Buffers.WriteTo() many times: https://golang.org/pkg/net/#Buffers.WriteTo

What did you expect to see?

Zero memory allocations.

What did you see instead?

Memory allocation for each call in this line: https://golang.org/src/net/fd_windows.go#L518 .

@ianlancetaylor ianlancetaylor changed the title Useless memory allocations. os: useless memory allocations on error Feb 21, 2017

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Feb 21, 2017

I'm not sure I understand. You are commenting on the allocation of the OpError value? That is how the os package handles all sorts of errors. What makes this one special?

@MichaelMonashev MichaelMonashev changed the title os: useless memory allocations on error useless memory allocations Feb 21, 2017

@MichaelMonashev MichaelMonashev changed the title useless memory allocations os: useless memory allocations on error Feb 21, 2017

@MichaelMonashev

This comment has been minimized.

Copy link
Author

commented Feb 21, 2017

nb := net.Buffers{}

	for {
		select {
		case <-done:
			return

		case b := <-s.bufCh:
			nb = append(nb, b.buf)

			for {
				select {
				case b = <-s.bufCh:
					nb = append(nb, b.buf)
					continue
				default:
				}
				break
			}

			_, err := nb.WriteTo(s.conn) // here I get memory allocation
		}
	}


(pprof) top
     42709  1.85% 94.37%      42709  1.85%  syscall.(*RawSockaddrAny).Sockaddr
     42704  1.85% 96.21%      85414  3.69%  net.(*UDPConn).readFrom
     42702  1.85% 98.06%      42707  1.85%  net.(*netFD).writeBuffers
     42673  1.84% 99.90%      42673  1.84%  net.ipToSockaddr
(pprof) list writeBuffers
ROUTINE ======================== net.(*netFD).writeBuffers in C:/Go/src/net/fd_windows.go
     42702      42707 (flat, cum)  1.85% of Total
         .          .    531:   if race.Enabled {
         .          .    532:           race.ReleaseMerge(unsafe.Pointer(&ioSync))
         .          .    533:   }
         .          .    534:   o := &fd.wop
         .          .    535:   o.InitBufs(buf)
     42702      42707    536:   n, err := wsrv.ExecIO(o, "WSASend", func(o *operation) error {
                   .          3   4e61b9: LEAL 0x542b80(IP), AX	                           C:/Go/src/net/fd_windows.go:536
                   .          .   4e61bf: MOVL AX, 0(SP)	                                  C:/Go/src/net/fd_windows.go:536
                   .          .   4e61c2: CALL runtime.newobject(SB)	                      C:/Go/src/net/fd_windows.go:536
               42702      42702   4e61c7: MOVL 0x4(SP), AX	                                C:/Go/src/net/fd_windows.go:536
                   .          .   4e61cb: MOVL AX, 0x40(SP)	                               C:/Go/src/net/fd_windows.go:536
                   .          .   4e61cf: LEAL 0x4f2f00(IP), CX	                           C:/Go/src/net/fd_windows.go:536
                   .          .   4e61d5: MOVL CX, 0(AX)	                                  C:/Go/src/net/fd_windows.go:536
                   .          .   4e61d7: TESTB AL, 0(AX)	                                 C:/Go/src/net/fd_windows.go:536
                   .          .   4e61d9: MOVL 0x71ee70(IP), CX	                           C:/Go/src/net/fd_windows.go:536
                   .          .   4e61df: LEAL 0x4(AX), DX	                                C:/Go/src/net/fd_windows.go:536
                   .          .   4e61e2: TESTL CX, CX	                                    C:/Go/src/net/fd_windows.go:536
                   .          .   4e61e4: JNE 0x4e635d	                                    C:/Go/src/net/fd_windows.go:536
                   .          .   4e61ea: MOVL 0x4c(SP), CX	                               C:/Go/src/net/fd_windows.go:536
                   .          .   4e61ee: MOVL CX, 0x4(AX)	                                C:/Go/src/net/fd_windows.go:536
                   .          .   4e6202: LEAL 0x5645f6(IP), BX	                           C:/Go/src/net/fd_windows.go:536
                   .          2   4e621d: MOVL 0x1c(SP), AX	                               C:/Go/src/net/fd_windows.go:536
                   .          .   4e6221: MOVL AX, 0x30(SP)	                               C:/Go/src/net/fd_windows.go:536
                   .          .   4e6225: MOVL 0x18(SP), CX	                               C:/Go/src/net/fd_windows.go:536
                   .          .   4e6229: MOVL CX, 0x2c(SP)	                               C:/Go/src/net/fd_windows.go:536
                   .          .   4e622d: MOVL 0x14(SP), DX	                               C:/Go/src/net/fd_windows.go:536
                   .          .   4e6231: MOVL DX, 0x20(SP)	                               C:/Go/src/net/fd_windows.go:536
                   .          .   4e635d: MOVL DX, 0(SP)	                                  C:/Go/src/net/fd_windows.go:536
                   .          .   4e6360: MOVL 0x4c(SP), CX	                               C:/Go/src/net/fd_windows.go:536
                   .          .   4e6364: MOVL CX, 0x4(SP)	                                C:/Go/src/net/fd_windows.go:536
                   .          .   4e6368: CALL runtime.writebarrierptr(SB)	                C:/Go/src/net/fd_windows.go:536
         .          .    537:           return syscall.WSASend(o.fd.sysfd, &o.bufs[0], uint32(len(*buf)), &o.qty, 0, &o.o, nil)
         .          .    538:   })
         .          .    539:   o.ClearBufs()
         .          .    540:   if _, ok := err.(syscall.Errno); ok {
         .          .    541:           err = os.NewSyscallError("wsasend", err)
(pprof)
@MichaelMonashev

This comment has been minimized.

Copy link
Author

commented Feb 21, 2017

The problem inside net package.

@MichaelMonashev

This comment has been minimized.

Copy link
Author

commented Feb 21, 2017

Anonymous function func(o *operation) error use external variables.

@MichaelMonashev MichaelMonashev changed the title os: useless memory allocations on error net: useless memory allocations on writeBuffers() Feb 21, 2017

@MichaelMonashev

This comment has been minimized.

Copy link
Author

commented Feb 21, 2017

May be this changes remove allocations:

- return syscall.WSASend(o.fd.sysfd, &o.bufs[0], uint32(len(*buf)), &o.qty, 0, &o.o, nil)
+ return syscall.WSASend(o.fd.sysfd, &o.bufs[0], uint32(len(o.bufs)), &o.qty, 0, &o.o, nil)

@MichaelMonashev

This comment has been minimized.

Copy link
Author

commented Feb 21, 2017

Original commit: e22b5ef

@ianlancetaylor ianlancetaylor changed the title net: useless memory allocations on writeBuffers() net: useless memory allocation in writeBuffers on Windows Feb 21, 2017

@ianlancetaylor ianlancetaylor added this to the Go1.9 milestone Feb 21, 2017

@alexbrainman

This comment has been minimized.

Copy link
Member

commented Feb 21, 2017

@MichaelMonashev that sounds reasonable - looks like my own oversight. Would you like to send a change as per https://golang.org/doc/contribute.html ? The change will need a new test to verify that bug is fixed - something similar to TestTCPReadWriteAllocs in net package.

Thank you.

Alex

@gopherbot

This comment has been minimized.

Copy link

commented May 7, 2017

CL https://golang.org/cl/42893 mentions this issue.

@gopherbot gopherbot closed this in ddcb975 May 8, 2017

@golang golang locked and limited conversation to collaborators May 8, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.