Skip to content

Commit

Permalink
Disable compression by default
Browse files Browse the repository at this point in the history
Closes #220 and #230
  • Loading branch information
nhooyr committed May 18, 2020
1 parent b453d3e commit 17cf0fe
Show file tree
Hide file tree
Showing 10 changed files with 71 additions and 46 deletions.
3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ go get nhooyr.io/websocket
- Minimal and idiomatic API
- First class [context.Context](https://blog.golang.org/context) support
- Fully passes the WebSocket [autobahn-testsuite](https://github.com/crossbario/autobahn-testsuite)
- [Single dependency](https://pkg.go.dev/nhooyr.io/websocket?tab=imports)
- [Zero dependencies](https://pkg.go.dev/nhooyr.io/websocket?tab=imports)
- JSON and protobuf helpers in the [wsjson](https://pkg.go.dev/nhooyr.io/websocket/wsjson) and [wspb](https://pkg.go.dev/nhooyr.io/websocket/wspb) subpackages
- Zero alloc reads and writes
- Concurrent writes
Expand Down Expand Up @@ -112,7 +112,6 @@ Advantages of nhooyr.io/websocket:
- Gorilla's implementation is slower and uses [unsafe](https://golang.org/pkg/unsafe/).
- Full [permessage-deflate](https://tools.ietf.org/html/rfc7692) compression extension support
- Gorilla only supports no context takeover mode
- We use [klauspost/compress](https://github.com/klauspost/compress) for much lower memory usage ([gorilla/websocket#203](https://github.com/gorilla/websocket/issues/203))
- [CloseRead](https://pkg.go.dev/nhooyr.io/websocket#Conn.CloseRead) helper ([gorilla/websocket#492](https://github.com/gorilla/websocket/issues/492))
- Actively maintained ([gorilla/websocket#370](https://github.com/gorilla/websocket/issues/370))

Expand Down
2 changes: 1 addition & 1 deletion accept.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ type AcceptOptions struct {
OriginPatterns []string

// CompressionMode controls the compression mode.
// Defaults to CompressionNoContextTakeover.
// Defaults to CompressionDisabled.
//
// See docs on CompressionMode for details.
CompressionMode CompressionMode
Expand Down
4 changes: 3 additions & 1 deletion accept_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ func TestAccept(t *testing.T) {
r.Header.Set("Sec-WebSocket-Key", "meow123")
r.Header.Set("Sec-WebSocket-Extensions", "permessage-deflate; harharhar")

_, err := Accept(w, r, nil)
_, err := Accept(w, r, &AcceptOptions{
CompressionMode: CompressionContextTakeover,
})
assert.Contains(t, err, `unsupported permessage-deflate parameter`)
})

Expand Down
4 changes: 3 additions & 1 deletion autobahn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ func TestAutobahn(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), time.Minute*5)
defer cancel()

c, _, err := websocket.Dial(ctx, fmt.Sprintf(wstestURL+"/runCase?case=%v&agent=main", i), nil)
c, _, err := websocket.Dial(ctx, fmt.Sprintf(wstestURL+"/runCase?case=%v&agent=main", i), &websocket.DialOptions{
CompressionMode: websocket.CompressionContextTakeover,
})
assert.Success(t, err)
err = wstest.EchoLoop(ctx, c)
t.Logf("echoLoop: %v", err)
Expand Down
60 changes: 37 additions & 23 deletions compress.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,49 +3,47 @@
package websocket

import (
"compress/flate"
"io"
"net/http"
"sync"

"github.com/klauspost/compress/flate"
)

// CompressionMode represents the modes available to the deflate extension.
// See https://tools.ietf.org/html/rfc7692
//
// A compatibility layer is implemented for the older deflate-frame extension used
// by safari. See https://tools.ietf.org/html/draft-tyoshino-hybi-websocket-perframe-deflate-06
// It will work the same in every way except that we cannot signal to the peer we
// want to use no context takeover on our side, we can only signal that they should.
// It is however currently disabled due to Safari bugs. See https://github.com/nhooyr/websocket/issues/218
type CompressionMode int

const (
// CompressionNoContextTakeover grabs a new flate.Reader and flate.Writer as needed
// for every message. This applies to both server and client side.
// CompressionDisabled disables the deflate extension.
//
// This means less efficient compression as the sliding window from previous messages
// will not be used but the memory overhead will be lower if the connections
// are long lived and seldom used.
// Use this if you are using a predominantly binary protocol with very
// little duplication in between messages or CPU and memory are more
// important than bandwidth.
//
// The message will only be compressed if greater than 512 bytes.
CompressionNoContextTakeover CompressionMode = iota
// This is the default.
CompressionDisabled CompressionMode = iota

// CompressionContextTakeover uses a flate.Reader and flate.Writer per connection.
// This enables reusing the sliding window from previous messages.
// CompressionContextTakeover uses a 32 kB sliding window and flate.Writer per connection.
// It reusing the sliding window from previous messages.
// As most WebSocket protocols are repetitive, this can be very efficient.
// It carries an overhead of 8 kB for every connection compared to CompressionNoContextTakeover.
// It carries an overhead of 32 kB + 1.2 MB for every connection compared to CompressionNoContextTakeover.
//
// Sometime in the future it will carry 65 kB overhead instead once https://github.com/golang/go/issues/36919
// is fixed.
//
// If the peer negotiates NoContextTakeover on the client or server side, it will be
// used instead as this is required by the RFC.
CompressionContextTakeover

// CompressionDisabled disables the deflate extension.
// CompressionNoContextTakeover grabs a new flate.Reader and flate.Writer as needed
// for every message. This applies to both server and client side.
//
// Use this if you are using a predominantly binary protocol with very
// little duplication in between messages or CPU and memory are more
// important than bandwidth.
CompressionDisabled
// This means less efficient compression as the sliding window from previous messages
// will not be used but the memory overhead will be lower if the connections
// are long lived and seldom used.
//
// The message will only be compressed if greater than 512 bytes.
CompressionNoContextTakeover
)

func (m CompressionMode) opts() *compressionOptions {
Expand Down Expand Up @@ -146,6 +144,22 @@ func putFlateReader(fr io.Reader) {
flateReaderPool.Put(fr)
}

var flateWriterPool sync.Pool

func getFlateWriter(w io.Writer) *flate.Writer {
fw, ok := flateWriterPool.Get().(*flate.Writer)
if !ok {
fw, _ = flate.NewWriter(w, flate.BestSpeed)
return fw
}
fw.Reset(w)
return fw
}

func putFlateWriter(w *flate.Writer) {
flateWriterPool.Put(w)
}

type slidingWindow struct {
buf []byte
}
Expand Down
4 changes: 2 additions & 2 deletions conn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestConn(t *testing.T) {
t.Parallel()

compressionMode := func() websocket.CompressionMode {
return websocket.CompressionMode(xrand.Int(int(websocket.CompressionDisabled) + 1))
return websocket.CompressionMode(xrand.Int(int(websocket.CompressionContextTakeover) + 1))
}

for i := 0; i < 5; i++ {
Expand Down Expand Up @@ -389,7 +389,7 @@ func BenchmarkConn(b *testing.B) {
mode: websocket.CompressionDisabled,
},
{
name: "compress",
name: "compressContextTakeover",
mode: websocket.CompressionContextTakeover,
},
{
Expand Down
2 changes: 1 addition & 1 deletion dial.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type DialOptions struct {
Subprotocols []string

// CompressionMode controls the compression mode.
// Defaults to CompressionNoContextTakeover.
// Defaults to CompressionDisabled.
//
// See docs on CompressionMode for details.
CompressionMode CompressionMode
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,5 @@ require (
github.com/golang/protobuf v1.3.5
github.com/google/go-cmp v0.4.0
github.com/gorilla/websocket v1.4.1
github.com/klauspost/compress v1.10.3
golang.org/x/time v0.0.0-20191024005414-555d28b269f0
)
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ github.com/gorilla/websocket v1.4.1 h1:q7AeDBpnBk8AogcD4DSag/Ukw/KV+YhzLj2bP5HvK
github.com/gorilla/websocket v1.4.1/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
github.com/json-iterator/go v1.1.9 h1:9yzud/Ht36ygwatGx56VwCZtlI/2AD15T1X2sjSuGns=
github.com/json-iterator/go v1.1.9/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4=
github.com/klauspost/compress v1.10.3 h1:OP96hzwJVBIHYU52pVTI6CczrxPvrGfgqF9N5eTO0Q8=
github.com/klauspost/compress v1.10.3/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
github.com/leodido/go-urn v1.2.0 h1:hpXL4XnriNwQ/ABnpepYM/1vCLWNDfUNts8dX3xTG6Y=
github.com/leodido/go-urn v1.2.0/go.mod h1:+8+nEpDfqqsY+g338gtMEUOtuK+4dEMhiQEgxpxOKII=
github.com/mattn/go-isatty v0.0.12 h1:wuysRhFDzyxgEmMf5xjvJ2M9dZoWAXNNr5LSBS7uHXY=
Expand Down
35 changes: 23 additions & 12 deletions write.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"io"
"time"

"github.com/klauspost/compress/flate"
"compress/flate"

"nhooyr.io/websocket/internal/errd"
)
Expand Down Expand Up @@ -76,8 +76,8 @@ type msgWriterState struct {
opcode opcode
flate bool

trimWriter *trimLastFourBytesWriter
dict slidingWindow
trimWriter *trimLastFourBytesWriter
flateWriter *flate.Writer
}

func newMsgWriterState(c *Conn) *msgWriterState {
Expand All @@ -96,7 +96,9 @@ func (mw *msgWriterState) ensureFlate() {
}
}

mw.dict.init(8192)
if mw.flateWriter == nil {
mw.flateWriter = getFlateWriter(mw.trimWriter)
}
mw.flate = true
}

Expand Down Expand Up @@ -153,6 +155,13 @@ func (mw *msgWriterState) reset(ctx context.Context, typ MessageType) error {
return nil
}

func (mw *msgWriterState) putFlateWriter() {
if mw.flateWriter != nil {
putFlateWriter(mw.flateWriter)
mw.flateWriter = nil
}
}

// Write writes the given bytes to the WebSocket connection.
func (mw *msgWriterState) Write(p []byte) (_ int, err error) {
err = mw.writeMu.lock(mw.ctx)
Expand All @@ -177,12 +186,7 @@ func (mw *msgWriterState) Write(p []byte) (_ int, err error) {
}

if mw.flate {
err = flate.StatelessDeflate(mw.trimWriter, p, false, mw.dict.buf)
if err != nil {
return 0, err
}
mw.dict.write(p)
return len(p), nil
return mw.flateWriter.Write(p)
}

return mw.write(p)
Expand All @@ -207,13 +211,20 @@ func (mw *msgWriterState) Close() (err error) {
}
defer mw.writeMu.unlock()

if mw.flate {
err = mw.flateWriter.Flush()
if err != nil {
return fmt.Errorf("failed to flush flate: %w", err)
}
}

_, err = mw.c.writeFrame(mw.ctx, true, mw.flate, mw.opcode, nil)
if err != nil {
return fmt.Errorf("failed to write fin frame: %w", err)
}

if mw.flate && !mw.flateContextTakeover() {
mw.dict.close()
mw.putFlateWriter()
}
mw.mu.unlock()
return nil
Expand All @@ -226,7 +237,7 @@ func (mw *msgWriterState) close() {
}

mw.writeMu.forceLock()
mw.dict.close()
mw.putFlateWriter()
}

func (c *Conn) writeControl(ctx context.Context, opcode opcode, p []byte) error {
Expand Down

0 comments on commit 17cf0fe

Please sign in to comment.