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.

Sign up for GitHub

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

Jump to bottom

net: useless memory allocation in writeBuffers on Windows #19222

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

net: useless memory allocation in writeBuffers on Windows #19222

MichaelMonashev opened this issue Feb 21, 2017 · 8 comments
Labels
FrozenDueToAge OS-Windows
Milestone
Go1.9

Comments

@MichaelMonashev
Copy link

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
Copy link
Contributor

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
Copy link
Author 

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
Copy link
Author

The problem inside net package.

@MichaelMonashev
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Member

@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
Copy link

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

@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.
Assignees
No one assigned
Labels
FrozenDueToAge OS-Windows
Projects
None yet
Milestone
Go1.9
Development

No branches or pull requests
4 participants
@ianlancetaylor @MichaelMonashev @gopherbot @alexbrainman