Skip to content

Can not do Zero alloc reads and writes. #354

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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
Contributor

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
Contributor

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
Contributor

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
Contributor

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
Contributor

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
Contributor

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
Contributor

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
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

3 participants