Browse code

Fix benchmarks and remove more unnecessary code.

Signed-off-by: Daniel Nephin <dnephin@docker.com>

Daniel Nephin authored on 2017/09/26 04:52:42
Showing 5 changed files
... ...
@@ -14,7 +14,7 @@ import (
14 14
 	"github.com/docker/docker/daemon/logger"
15 15
 	"github.com/docker/docker/daemon/logger/loggerutils"
16 16
 	"github.com/docker/docker/pkg/jsonlog"
17
-	"github.com/docker/go-units"
17
+	units "github.com/docker/go-units"
18 18
 	"github.com/pkg/errors"
19 19
 	"github.com/sirupsen/logrus"
20 20
 )
... ...
@@ -106,10 +106,8 @@ func writeMessageBuf(w io.Writer, m *logger.Message, extra json.RawMessage, buf
106 106
 		return err
107 107
 	}
108 108
 	logger.PutMessage(m)
109
-	if _, err := w.Write(buf.Bytes()); err != nil {
110
-		return errors.Wrap(err, "error writing log entry")
111
-	}
112
-	return nil
109
+	_, err := w.Write(buf.Bytes())
110
+	return errors.Wrap(err, "error writing log entry")
113 111
 }
114 112
 
115 113
 func marshalMessage(msg *logger.Message, extra json.RawMessage, buf *bytes.Buffer) error {
... ...
@@ -1,6 +1,7 @@
1 1
 package jsonfilelog
2 2
 
3 3
 import (
4
+	"bytes"
4 5
 	"encoding/json"
5 6
 	"io/ioutil"
6 7
 	"os"
... ...
@@ -12,6 +13,8 @@ import (
12 12
 
13 13
 	"github.com/docker/docker/daemon/logger"
14 14
 	"github.com/docker/docker/pkg/jsonlog"
15
+	"github.com/gotestyourself/gotestyourself/fs"
16
+	"github.com/stretchr/testify/require"
15 17
 )
16 18
 
17 19
 func TestJSONFileLogger(t *testing.T) {
... ...
@@ -54,36 +57,38 @@ func TestJSONFileLogger(t *testing.T) {
54 54
 	}
55 55
 }
56 56
 
57
-func BenchmarkJSONFileLogger(b *testing.B) {
58
-	cid := "a7317399f3f857173c6179d44823594f8294678dea9999662e5c625b5a1c7657"
59
-	tmp, err := ioutil.TempDir("", "docker-logger-")
60
-	if err != nil {
61
-		b.Fatal(err)
62
-	}
63
-	defer os.RemoveAll(tmp)
64
-	filename := filepath.Join(tmp, "container.log")
65
-	l, err := New(logger.Info{
66
-		ContainerID: cid,
67
-		LogPath:     filename,
57
+func BenchmarkJSONFileLoggerLog(b *testing.B) {
58
+	tmp := fs.NewDir(b, "bench-jsonfilelog")
59
+	defer tmp.Remove()
60
+
61
+	jsonlogger, err := New(logger.Info{
62
+		ContainerID: "a7317399f3f857173c6179d44823594f8294678dea9999662e5c625b5a1c7657",
63
+		LogPath:     tmp.Join("container.log"),
64
+		Config: map[string]string{
65
+			"labels": "first,second",
66
+		},
67
+		ContainerLabels: map[string]string{
68
+			"first":  "label_value",
69
+			"second": "label_foo",
70
+		},
68 71
 	})
69
-	if err != nil {
70
-		b.Fatal(err)
71
-	}
72
-	defer l.Close()
72
+	require.NoError(b, err)
73
+	defer jsonlogger.Close()
73 74
 
74
-	testLine := "Line that thinks that it is log line from docker\n"
75
-	msg := &logger.Message{Line: []byte(testLine), Source: "stderr", Timestamp: time.Now().UTC()}
76
-	jsonlog, err := (&jsonlog.JSONLog{Log: string(msg.Line) + "\n", Stream: msg.Source, Created: msg.Timestamp}).MarshalJSON()
77
-	if err != nil {
78
-		b.Fatal(err)
75
+	msg := &logger.Message{
76
+		Line:      []byte("Line that thinks that it is log line from docker\n"),
77
+		Source:    "stderr",
78
+		Timestamp: time.Now().UTC(),
79 79
 	}
80
-	b.SetBytes(int64(len(jsonlog)+1) * 30)
80
+
81
+	buf := bytes.NewBuffer(nil)
82
+	require.NoError(b, marshalMessage(msg, jsonlogger.(*JSONFileLogger).extra, buf))
83
+	b.SetBytes(int64(buf.Len()))
84
+
81 85
 	b.ResetTimer()
82 86
 	for i := 0; i < b.N; i++ {
83
-		for j := 0; j < 30; j++ {
84
-			if err := l.Log(msg); err != nil {
85
-				b.Fatal(err)
86
-			}
87
+		if err := jsonlogger.Log(msg); err != nil {
88
+			b.Fatal(err)
87 89
 		}
88 90
 	}
89 91
 }
... ...
@@ -200,50 +205,3 @@ func TestJSONFileLoggerWithLabelsEnv(t *testing.T) {
200 200
 		t.Fatalf("Wrong log attrs: %q, expected %q", extra, expected)
201 201
 	}
202 202
 }
203
-
204
-func BenchmarkJSONFileLoggerWithReader(b *testing.B) {
205
-	b.StopTimer()
206
-	b.ResetTimer()
207
-	cid := "a7317399f3f857173c6179d44823594f8294678dea9999662e5c625b5a1c7657"
208
-	dir, err := ioutil.TempDir("", "json-logger-bench")
209
-	if err != nil {
210
-		b.Fatal(err)
211
-	}
212
-	defer os.RemoveAll(dir)
213
-
214
-	l, err := New(logger.Info{
215
-		ContainerID: cid,
216
-		LogPath:     filepath.Join(dir, "container.log"),
217
-	})
218
-	if err != nil {
219
-		b.Fatal(err)
220
-	}
221
-	defer l.Close()
222
-	msg := &logger.Message{Line: []byte("line"), Source: "src1"}
223
-	jsonlog, err := (&jsonlog.JSONLog{Log: string(msg.Line) + "\n", Stream: msg.Source, Created: msg.Timestamp}).MarshalJSON()
224
-	if err != nil {
225
-		b.Fatal(err)
226
-	}
227
-	b.SetBytes(int64(len(jsonlog)+1) * 30)
228
-
229
-	b.StartTimer()
230
-
231
-	go func() {
232
-		for i := 0; i < b.N; i++ {
233
-			for j := 0; j < 30; j++ {
234
-				l.Log(msg)
235
-			}
236
-		}
237
-		l.Close()
238
-	}()
239
-
240
-	lw := l.(logger.LogReader).ReadLogs(logger.ReadConfig{Follow: true})
241
-	watchClose := lw.WatchClose()
242
-	for {
243
-		select {
244
-		case <-lw.Msg:
245
-		case <-watchClose:
246
-			return
247
-		}
248
-	}
249
-}
250 203
new file mode 100644
... ...
@@ -0,0 +1,65 @@
0
+package jsonfilelog
1
+
2
+import (
3
+	"testing"
4
+
5
+	"bytes"
6
+	"time"
7
+
8
+	"github.com/docker/docker/daemon/logger"
9
+	"github.com/gotestyourself/gotestyourself/fs"
10
+	"github.com/stretchr/testify/require"
11
+)
12
+
13
+func BenchmarkJSONFileLoggerReadLogs(b *testing.B) {
14
+	tmp := fs.NewDir(b, "bench-jsonfilelog")
15
+	defer tmp.Remove()
16
+
17
+	jsonlogger, err := New(logger.Info{
18
+		ContainerID: "a7317399f3f857173c6179d44823594f8294678dea9999662e5c625b5a1c7657",
19
+		LogPath:     tmp.Join("container.log"),
20
+		Config: map[string]string{
21
+			"labels": "first,second",
22
+		},
23
+		ContainerLabels: map[string]string{
24
+			"first":  "label_value",
25
+			"second": "label_foo",
26
+		},
27
+	})
28
+	require.NoError(b, err)
29
+	defer jsonlogger.Close()
30
+
31
+	msg := &logger.Message{
32
+		Line:      []byte("Line that thinks that it is log line from docker\n"),
33
+		Source:    "stderr",
34
+		Timestamp: time.Now().UTC(),
35
+	}
36
+
37
+	buf := bytes.NewBuffer(nil)
38
+	require.NoError(b, marshalMessage(msg, jsonlogger.(*JSONFileLogger).extra, buf))
39
+	b.SetBytes(int64(buf.Len()))
40
+
41
+	b.ResetTimer()
42
+
43
+	chError := make(chan error, b.N+1)
44
+	go func() {
45
+		for i := 0; i < b.N; i++ {
46
+			chError <- jsonlogger.Log(msg)
47
+		}
48
+		chError <- jsonlogger.Close()
49
+	}()
50
+
51
+	lw := jsonlogger.(*JSONFileLogger).ReadLogs(logger.ReadConfig{Follow: true})
52
+	watchClose := lw.WatchClose()
53
+	for {
54
+		select {
55
+		case <-lw.Msg:
56
+		case <-watchClose:
57
+			return
58
+		case err := <-chError:
59
+			if err != nil {
60
+				b.Fatal(err)
61
+			}
62
+		}
63
+	}
64
+}
... ...
@@ -4,8 +4,7 @@ import (
4 4
 	"time"
5 5
 )
6 6
 
7
-// JSONLog represents a log message, typically a single entry from a given log stream.
8
-// JSONLogs can be easily serialized to and from JSON and support custom formatting.
7
+// JSONLog is a log message, typically a single entry from a given log stream.
9 8
 type JSONLog struct {
10 9
 	// Log is the log message
11 10
 	Log string `json:"log,omitempty"`
... ...
@@ -17,8 +17,8 @@ type JSONLogs struct {
17 17
 	RawAttrs json.RawMessage `json:"attrs,omitempty"`
18 18
 }
19 19
 
20
-// MarshalJSONBuf is based on the same method from JSONLog
21
-// It has been modified to take into account the necessary changes.
20
+// MarshalJSONBuf is an optimized JSON marshaller that avoids reflection
21
+// and unnecessary allocation.
22 22
 func (mj *JSONLogs) MarshalJSONBuf(buf *bytes.Buffer) error {
23 23
 	var first = true
24 24
 
... ...
@@ -35,7 +35,7 @@ func (mj *JSONLogs) MarshalJSONBuf(buf *bytes.Buffer) error {
35 35
 			buf.WriteString(`,`)
36 36
 		}
37 37
 		buf.WriteString(`"stream":`)
38
-		ffjsonWriteJSONString(buf, mj.Stream)
38
+		ffjsonWriteJSONBytesAsString(buf, []byte(mj.Stream))
39 39
 	}
40 40
 	if len(mj.RawAttrs) > 0 {
41 41
 		if first {
... ...
@@ -61,72 +61,6 @@ func (mj *JSONLogs) MarshalJSONBuf(buf *bytes.Buffer) error {
61 61
 	return nil
62 62
 }
63 63
 
64
-func ffjsonWriteJSONString(buf *bytes.Buffer, s string) {
65
-	const hex = "0123456789abcdef"
66
-
67
-	buf.WriteByte('"')
68
-	start := 0
69
-	for i := 0; i < len(s); {
70
-		if b := s[i]; b < utf8.RuneSelf {
71
-			if 0x20 <= b && b != '\\' && b != '"' && b != '<' && b != '>' && b != '&' {
72
-				i++
73
-				continue
74
-			}
75
-			if start < i {
76
-				buf.WriteString(s[start:i])
77
-			}
78
-			switch b {
79
-			case '\\', '"':
80
-				buf.WriteByte('\\')
81
-				buf.WriteByte(b)
82
-			case '\n':
83
-				buf.WriteByte('\\')
84
-				buf.WriteByte('n')
85
-			case '\r':
86
-				buf.WriteByte('\\')
87
-				buf.WriteByte('r')
88
-			default:
89
-
90
-				buf.WriteString(`\u00`)
91
-				buf.WriteByte(hex[b>>4])
92
-				buf.WriteByte(hex[b&0xF])
93
-			}
94
-			i++
95
-			start = i
96
-			continue
97
-		}
98
-		c, size := utf8.DecodeRuneInString(s[i:])
99
-		if c == utf8.RuneError && size == 1 {
100
-			if start < i {
101
-				buf.WriteString(s[start:i])
102
-			}
103
-			buf.WriteString(`\ufffd`)
104
-			i += size
105
-			start = i
106
-			continue
107
-		}
108
-
109
-		if c == '\u2028' || c == '\u2029' {
110
-			if start < i {
111
-				buf.WriteString(s[start:i])
112
-			}
113
-			buf.WriteString(`\u202`)
114
-			buf.WriteByte(hex[c&0xF])
115
-			i += size
116
-			start = i
117
-			continue
118
-		}
119
-		i += size
120
-	}
121
-	if start < len(s) {
122
-		buf.WriteString(s[start:])
123
-	}
124
-	buf.WriteByte('"')
125
-}
126
-
127
-// This is based on ffjsonWriteJSONString. It has been changed
128
-// to accept a string passed as a slice of bytes.
129
-// TODO: remove duplication with ffjsonWriteJSONString
130 64
 func ffjsonWriteJSONBytesAsString(buf *bytes.Buffer, s []byte) {
131 65
 	const hex = "0123456789abcdef"
132 66