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

Can not do Zero alloc reads and writes. #354

Closed
bronze1man opened this issue Nov 3, 2022 · 12 comments
Closed

Can not do Zero alloc reads and writes. #354

bronze1man opened this issue Nov 3, 2022 · 12 comments

Comments

@bronze1man
Copy link

bronze1man commented Nov 3, 2022

I wrote some tests, here is my code:

package main

import (
	"net/http"
	"io"
	"fmt"
	"nhooyr.io/websocket"
	"context"
	"time"
	"bytes"
	"runtime"

)


func main(){
	go func(){
		http.ListenAndServe(`127.0.0.1:9098`,echoServer{})
	}()
	time.Sleep(time.Millisecond*10)
	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
	defer cancel()
	c, _, err := websocket.Dial(ctx, `http://127.0.0.1:9098`, &websocket.DialOptions{
		Subprotocols: []string{"echo"},
	})
	defer c.Close(websocket.StatusNormalClosure,``)
	ctx = context.Background()
	inList:=[]byte{1}
	buf:=make([]byte,1024)
	c2:=websocket.NetConn(ctx,c,websocket.MessageBinary)
	runLoopOnceFn:=func(){
		_,err=c2.Write(inList)
		if err!=nil{
			panic(err)
		}
		for {
			nr,err := c2.Read(buf)
			if err!=nil{
				panic(err)
			}
			if nr==0{
				continue
			}
			if nr!=1 || bytes.Equal(buf[:nr],inList)==false{
				panic(`b7wdgy9dyz`)
			}
			return
		}
	}
	for i:=0;i<1024;i++{
		runLoopOnceFn()
	}
	var m runtime.MemStats
	runtime.ReadMemStats(&m)
	startAllocSize:=m.TotalAlloc
	startAllocNum:=m.Mallocs
	const loopNum = 1024
	for i:=0;i<loopNum;i++{
		runLoopOnceFn()
	}
	runtime.ReadMemStats(&m)
	endAllocSize:=m.TotalAlloc
	endAllocNum:=m.Mallocs
	fmt.Println("AllocSize",float64(endAllocSize-startAllocSize)/float64(loopNum))
	fmt.Println("AllocNum",float64(endAllocNum-startAllocNum)/float64(loopNum))

}

// echoServer is the WebSocket echo server implementation.
// It ensures the client speaks the echo subprotocol and
// only allows one message every 100ms with a 10 message burst.
type echoServer struct {
	// logf controls where logs are sent.
	logf func(f string, v ...interface{})
}

func (s echoServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
	c, err := websocket.Accept(w, r, &websocket.AcceptOptions{
		Subprotocols: []string{"echo"},
	})
	if err != nil {
		s.logf("%v", err)
		return
	}
	defer c.Close(websocket.StatusInternalError, "the sky is falling")
	if c.Subprotocol() != "echo" {
		c.Close(websocket.StatusPolicyViolation, "client must speak the echo subprotocol")
		return
	}
	for {
		err = echo(r.Context(), c)
		if websocket.CloseStatus(err) == websocket.StatusNormalClosure {
			return
		}
		if err != nil {
			s.logf("failed to echo with %v: %v", r.RemoteAddr, err)
			return
		}
	}
}

// echo reads from the WebSocket connection and then writes
// the received message back to it.
// The entire function has 10s to complete.
func echo(ctx context.Context, c *websocket.Conn) error {
	ctx, cancel := context.WithTimeout(ctx, time.Second*10)
	defer cancel()

	typ, r, err := c.Reader(ctx)
	if err != nil {
		return err
	}

	w, err := c.Writer(ctx, typ)
	if err != nil {
		return err
	}

	_, err = io.Copy(w, r)
	if err != nil {
		return fmt.Errorf("failed to io.Copy: %w", err)
	}

	err = w.Close()
	return err
}
  • I got result:
AllocSize 33131.9296875
AllocNum 9.0244140625
  • here is alloc place list in one runLoopOnceFn call (with my internal tool, may open source soon)
=========================================================
allocSize: 32.00KB allocNum: 1.0000B percentSize: 99.83%
type: uint8
/usr/local/go/src/runtime/malloc.go:1121
/usr/local/go/src/runtime/slice.go:103
/usr/local/go/src/io/io.go:424
/usr/local/go/src/io/io.go:386
main.go:119
main.go:91
/usr/local/go/src/net/http/server.go:2947
/usr/local/go/src/net/http/server.go:1991
/usr/local/go/src/net/http/server.go:3102
/usr/local/go/src/runtime/asm_amd64.s:1594
/usr/local/go/src/net/http/server.go:3102
=========================================================
allocSize: 24.000B allocNum: 1.0000B percentSize:  0.07%
type: websocket.CloseError
/usr/local/go/src/runtime/malloc.go:1121
/usr/local/go/src/runtime/malloc.go:1192
src/nhooyr.io/websocket/close.go:71
main.go:92
/usr/local/go/src/net/http/server.go:2947
/usr/local/go/src/net/http/server.go:1991
/usr/local/go/src/net/http/server.go:3102
/usr/local/go/src/runtime/asm_amd64.s:1594
/usr/local/go/src/net/http/server.go:3102
=========================================================
allocSize: 16.000B allocNum: 1.0000B percentSize:  0.05%
type: websocket.msgWriter
/usr/local/go/src/runtime/malloc.go:1121
/usr/local/go/src/runtime/malloc.go:1192
src/nhooyr.io/websocket/write.go:115
src/nhooyr.io/websocket/write.go:122
src/nhooyr.io/websocket/write.go:42
src/nhooyr.io/websocket/netconn.go:84
main.go:32
main.go:66
/usr/local/go/src/runtime/proc.go:250
/usr/local/go/src/runtime/asm_amd64.s:1594
=========================================================
allocSize: 16.000B allocNum: 1.0000B percentSize:  0.05%
type: websocket.msgWriter
/usr/local/go/src/runtime/malloc.go:1121
/usr/local/go/src/runtime/malloc.go:1192
src/nhooyr.io/websocket/write.go:115
src/nhooyr.io/websocket/write.go:28
main.go:114
main.go:91
/usr/local/go/src/net/http/server.go:2947
/usr/local/go/src/net/http/server.go:1991
/usr/local/go/src/net/http/server.go:3102
/usr/local/go/src/runtime/asm_amd64.s:1594
/usr/local/go/src/net/http/server.go:3102
@bronze1man
Copy link
Author

bronze1man commented Nov 3, 2022

Use my internal tool, I find that all the allocs is from Conn.Writer/io.Copy in serverside code。

@bronze1man bronze1man reopened this Nov 3, 2022
@bronze1man
Copy link
Author

bronze1man commented Nov 3, 2022

Then I have tried websocket.NetConn on both side, then I got:

allocNum: 2.0000B allocSize: 32.000B entryNum: 2.0000B
=========================================================
allocSize: 16.000B allocNum: 1.0000B percentSize: 50.00%
type: websocket.msgWriter
/usr/local/go/src/runtime/malloc.go:1121
/usr/local/go/src/runtime/malloc.go:1192
src/nhooyr.io/websocket/write.go:115
src/nhooyr.io/websocket/write.go:122
src/nhooyr.io/websocket/write.go:42
src/nhooyr.io/websocket/netconn.go:84
=========================================================
allocSize: 16.000B allocNum: 1.0000B percentSize: 50.00%
type: websocket.msgWriter
/usr/local/go/src/runtime/malloc.go:1121
/usr/local/go/src/runtime/malloc.go:1192
src/nhooyr.io/websocket/write.go:115
src/nhooyr.io/websocket/write.go:122
src/nhooyr.io/websocket/write.go:42
src/nhooyr.io/websocket/netconn.go:84
/usr/local/go/src/io/io.go:429
/usr/local/go/src/io/io.go:386

It is also not zero alloc .
I can not find any public interface to read and write with zero alloc right now.

@bronze1man
Copy link
Author

bronze1man commented Nov 3, 2022

zero alloc read/write message modify over github.com/nhooyr/websocket

  • Add ReadNoAlloc ,make calller easier.(reader can reuse the bufW)
func (c *Conn) ReadNoAlloc(ctx context.Context, bufW *bytes.Buffer) (MessageType, error) {
	typ, r, err := c.Reader(ctx)
	if err != nil {
		return 0, err
	}
	_,err=bufW.ReadFrom(r)
	return typ,err
}
  • Modify conn.write, delete that msgWriter alloc:
func (c *Conn) write(ctx context.Context, typ MessageType, p []byte) (int, error) {
	err := c.msgWriterState.reset(ctx,typ) //  c.msgWriterState.mu.lock() is inside.
	if err != nil {
		return 0, err
	}
	if !c.flate() {
		defer c.msgWriterState.mu.unlock()
		return c.writeFrame(ctx, true, false, c.msgWriterState.opcode, p)
	}
	n, err := c.msgWriterState.Write(p)
	if err != nil {
		return n, err
	}
	err = c.msgWriterState.Close() // c.msgWriterState.mu.unlock() is inside.
	return n, err
}

This change will make "Concurrent writes" invalid....

@bronze1man
Copy link
Author

So I comfirmed that it can not zero-alloced write message right now. It can zero-alloced read message.
So the Highlights in README.md is wrong.

@bronze1man bronze1man changed the title [doc] please document how to do Zero alloc reads and writes. Can not do Zero alloc reads and writes. Nov 3, 2022
@nhooyr
Copy link
Owner

nhooyr commented Mar 5, 2023

Will test.

@timo-klarshift
Copy link

@nhooyr can you please clarify if its correct whats stated in the README or not?

@nhooyr nhooyr added this to the v1.9.0 milestone Sep 28, 2023
@nhooyr
Copy link
Owner

nhooyr commented Sep 28, 2023

To the best of my knowledge it was correct when I wrote it but it's been a few years now so I need to retest everything I'm claiming in the README.

@nhooyr nhooyr modified the milestones: v1.9.0, v1.8.8 Oct 13, 2023
@nhooyr
Copy link
Owner

nhooyr commented Oct 13, 2023

Ah, yes the README should be clarified to indicate that only Conn.Write does a zero alloc write. Though I can change this for v1.8.8.

Reads are zero alloc though as you noticed.

@nhooyr
Copy link
Owner

nhooyr commented Oct 13, 2023

I added a script to make testing this stuff much easier. See ci/bench.sh

 ./ci/bench.sh -bench=/disabledCompress
go tool pprof ci/out/prof.mem
(pprof) top
Showing nodes accounting for 6353.34kB, 100% of 6353.34kB total
Showing top 10 nodes out of 24
      flat  flat%   sum%        cum   cum%
 2048.03kB 32.24% 32.24%  2048.03kB 32.24%  nhooyr.io/websocket.(*Conn).writer
 1762.94kB 27.75% 59.98%  1762.94kB 27.75%  runtime/pprof.StartCPUProfile
  902.59kB 14.21% 74.19%  2030.26kB 31.96%  compress/flate.NewWriter (inline)
  583.01kB  9.18% 83.37%   583.01kB  9.18%  compress/flate.newDeflateFast (inline)
  544.67kB  8.57% 91.94%  1127.67kB 17.75%  compress/flate.(*compressor).init
  512.11kB  8.06%   100%   512.11kB  8.06%  runtime/pprof.allFrames
         0     0%   100%  2030.26kB 31.96%  compress/gzip.(*Writer).Write
         0     0%   100%  1762.94kB 27.75%  main.main
         0     0%   100%  1024.02kB 16.12%  nhooyr.io/websocket.(*Conn).Write
         0     0%   100%  1024.02kB 16.12%  nhooyr.io/websocket.(*Conn).Writer

The other allocations are red herrings from pprof itself. The first one is the one you noticed as well.

@nhooyr
Copy link
Owner

nhooyr commented Oct 13, 2023

The reason it does an alloc anyway btw is to prevent closed writers from being used improperly. If I remove the allocation, then a writer obtained, written to and closed, will be open again when someone else is writing as there will only ever be one Writer pointer.

@nhooyr
Copy link
Owner

nhooyr commented Oct 13, 2023

But I think that's ok, quite unlikely for writers to be used after closed in most code I think as you have to call Close and so you have to be clear of its the lifetime.

nhooyr added a commit that referenced this issue Oct 14, 2023
@nhooyr
Copy link
Owner

nhooyr commented Oct 14, 2023

Fixed in dev!

@nhooyr nhooyr closed this as completed Oct 14, 2023
nhooyr added a commit that referenced this issue Oct 14, 2023
nhooyr added a commit that referenced this issue Oct 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants