From 6f80fcd870b53ebfd6fd3f3525101e7022c6c6b4 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 20 Mar 2017 00:02:07 +0000 Subject: [PATCH 1/4] Use faster mapRead function everywhere --- report/counters.go | 13 ++++++++----- report/edge_metadatas.go | 13 ++++++++----- report/sets.go | 13 ++++++++----- 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/report/counters.go b/report/counters.go index 48919ae60..82d2e92ef 100644 --- a/report/counters.go +++ b/report/counters.go @@ -160,11 +160,14 @@ func (c *Counters) CodecEncodeSelf(encoder *codec.Encoder) { // CodecDecodeSelf implements codec.Selfer func (c *Counters) CodecDecodeSelf(decoder *codec.Decoder) { - in := map[string]int{} - if err := decoder.Decode(&in); err != nil { - return - } - *c = Counters{}.fromIntermediate(in) + out := mapRead(decoder, func(isNil bool) interface{} { + var value int + if !isNil { + decoder.Decode(&value) + } + return value + }) + *c = Counters{out} } // MarshalJSON shouldn't be used, use CodecEncodeSelf instead diff --git a/report/edge_metadatas.go b/report/edge_metadatas.go index fd8d7a448..2609d8e85 100644 --- a/report/edge_metadatas.go +++ b/report/edge_metadatas.go @@ -177,11 +177,14 @@ func (c *EdgeMetadatas) CodecEncodeSelf(encoder *codec.Encoder) { // CodecDecodeSelf implements codec.Selfer func (c *EdgeMetadatas) CodecDecodeSelf(decoder *codec.Decoder) { - in := map[string]EdgeMetadata{} - if err := decoder.Decode(&in); err != nil { - return - } - *c = EdgeMetadatas{}.fromIntermediate(in) + out := mapRead(decoder, func(isNil bool) interface{} { + var value EdgeMetadata + if !isNil { + decoder.Decode(&value) + } + return value + }) + *c = EdgeMetadatas{out} } // MarshalJSON shouldn't be used, use CodecEncodeSelf instead diff --git a/report/sets.go b/report/sets.go index bd363e95e..821d6a223 100644 --- a/report/sets.go +++ b/report/sets.go @@ -177,11 +177,14 @@ func (s *Sets) CodecEncodeSelf(encoder *codec.Encoder) { // CodecDecodeSelf implements codec.Selfer func (s *Sets) CodecDecodeSelf(decoder *codec.Decoder) { - in := map[string]StringSet{} - if err := decoder.Decode(&in); err != nil { - return - } - *s = Sets{}.fromIntermediate(in) + out := mapRead(decoder, func(isNil bool) interface{} { + var value StringSet + if !isNil { + decoder.Decode(&value) + } + return value + }) + *s = Sets{out} } // MarshalJSON shouldn't be used, use CodecEncodeSelf instead From b3f53a7a81c9ac558c76aaef599083da6acafdf2 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 20 Mar 2017 22:52:13 +0000 Subject: [PATCH 2/4] Call CodecDecodeSelf() instead of Decode() This avoids a runtime type lookup, so goes a little faster. Also having less recursion makes it easier to interpret profiles. --- extras/generate_latest_map | 5 +++-- report/controls.go | 5 ++--- report/edge_metadatas.go | 3 ++- report/latest_map_generated.go | 10 ++++++---- report/marshal.go | 7 +++++++ report/metrics.go | 5 ++--- 6 files changed, 22 insertions(+), 13 deletions(-) diff --git a/extras/generate_latest_map b/extras/generate_latest_map index 1639bb15b..05051c3e8 100755 --- a/extras/generate_latest_map +++ b/extras/generate_latest_map @@ -47,6 +47,7 @@ function generate_latest_map() { type ${entry_type} struct { Timestamp time.Time ${json_timestamp} Value ${data_type} ${json_value} + dummySelfer } // String returns the StringLatestEntry's string representation. @@ -122,7 +123,7 @@ function generate_latest_map() { if m.Map == nil { m.Map = ps.NewMap() } - return ${latest_map_type}{m.Map.Set(key, &${entry_type}{timestamp, value})} + return ${latest_map_type}{m.Map.Set(key, &${entry_type}{Timestamp: timestamp, Value: value})} } // Delete the value for the given key. @@ -178,7 +179,7 @@ function generate_latest_map() { out := mapRead(decoder, func(isNil bool) interface{} { value := &${entry_type}{} if !isNil { - decoder.Decode(value) + value.CodecDecodeSelf(decoder) } return value }) diff --git a/report/controls.go b/report/controls.go index 85eed408b..02e87a5b8 100644 --- a/report/controls.go +++ b/report/controls.go @@ -91,6 +91,7 @@ func (nc NodeControls) Add(ids ...string) NodeControls { type wireNodeControls struct { Timestamp string `json:"timestamp,omitempty"` Controls StringSet `json:"controls,omitempty"` + dummySelfer } // CodecEncodeSelf implements codec.Selfer @@ -104,9 +105,7 @@ func (nc *NodeControls) CodecEncodeSelf(encoder *codec.Encoder) { // CodecDecodeSelf implements codec.Selfer func (nc *NodeControls) CodecDecodeSelf(decoder *codec.Decoder) { in := wireNodeControls{} - if err := decoder.Decode(&in); err != nil { - return - } + in.CodecDecodeSelf(decoder) *nc = NodeControls{ Timestamp: parseTime(in.Timestamp), Controls: in.Controls, diff --git a/report/edge_metadatas.go b/report/edge_metadatas.go index 2609d8e85..13b0d40c0 100644 --- a/report/edge_metadatas.go +++ b/report/edge_metadatas.go @@ -180,7 +180,7 @@ func (c *EdgeMetadatas) CodecDecodeSelf(decoder *codec.Decoder) { out := mapRead(decoder, func(isNil bool) interface{} { var value EdgeMetadata if !isNil { - decoder.Decode(&value) + value.CodecDecodeSelf(decoder) } return value }) @@ -221,6 +221,7 @@ type EdgeMetadata struct { IngressPacketCount *uint64 `json:"ingress_packet_count,omitempty"` EgressByteCount *uint64 `json:"egress_byte_count,omitempty"` // Transport layer IngressByteCount *uint64 `json:"ingress_byte_count,omitempty"` // Transport layer + dummySelfer } // String returns a string representation of this EdgeMetadata diff --git a/report/latest_map_generated.go b/report/latest_map_generated.go index 1786b0a46..7192cd7d6 100644 --- a/report/latest_map_generated.go +++ b/report/latest_map_generated.go @@ -14,6 +14,7 @@ import ( type stringLatestEntry struct { Timestamp time.Time `json:"timestamp"` Value string `json:"value"` + dummySelfer } // String returns the StringLatestEntry's string representation. @@ -89,7 +90,7 @@ func (m StringLatestMap) Set(key string, timestamp time.Time, value string) Stri if m.Map == nil { m.Map = ps.NewMap() } - return StringLatestMap{m.Map.Set(key, &stringLatestEntry{timestamp, value})} + return StringLatestMap{m.Map.Set(key, &stringLatestEntry{Timestamp: timestamp, Value: value})} } // Delete the value for the given key. @@ -145,7 +146,7 @@ func (m *StringLatestMap) CodecDecodeSelf(decoder *codec.Decoder) { out := mapRead(decoder, func(isNil bool) interface{} { value := &stringLatestEntry{} if !isNil { - decoder.Decode(value) + value.CodecDecodeSelf(decoder) } return value }) @@ -165,6 +166,7 @@ func (*StringLatestMap) UnmarshalJSON(b []byte) error { type nodeControlDataLatestEntry struct { Timestamp time.Time `json:"timestamp"` Value NodeControlData `json:"value"` + dummySelfer } // String returns the StringLatestEntry's string representation. @@ -240,7 +242,7 @@ func (m NodeControlDataLatestMap) Set(key string, timestamp time.Time, value Nod if m.Map == nil { m.Map = ps.NewMap() } - return NodeControlDataLatestMap{m.Map.Set(key, &nodeControlDataLatestEntry{timestamp, value})} + return NodeControlDataLatestMap{m.Map.Set(key, &nodeControlDataLatestEntry{Timestamp: timestamp, Value: value})} } // Delete the value for the given key. @@ -296,7 +298,7 @@ func (m *NodeControlDataLatestMap) CodecDecodeSelf(decoder *codec.Decoder) { out := mapRead(decoder, func(isNil bool) interface{} { value := &nodeControlDataLatestEntry{} if !isNil { - decoder.Decode(value) + value.CodecDecodeSelf(decoder) } return value }) diff --git a/report/marshal.go b/report/marshal.go index 1e013761a..b21e01a17 100644 --- a/report/marshal.go +++ b/report/marshal.go @@ -10,6 +10,13 @@ import ( "github.com/ugorji/go/codec" ) +// Include this in a struct to be able to call CodecDecodeSelf() before code generation +type dummySelfer struct{} + +func (s *dummySelfer) CodecDecodeSelf(decoder *codec.Decoder) { + panic("This shouldn't happen: perhaps something has gone wrong in code generation?") +} + // WriteBinary writes a Report as a gzipped msgpack. func (rep Report) WriteBinary(w io.Writer, compressionLevel int) error { gzwriter, err := gzip.NewWriterLevel(w, compressionLevel) diff --git a/report/metrics.go b/report/metrics.go index 65ff5a35e..ea1122c92 100644 --- a/report/metrics.go +++ b/report/metrics.go @@ -225,6 +225,7 @@ type WireMetrics struct { Max float64 `json:"max"` First string `json:"first,omitempty"` Last string `json:"last,omitempty"` + dummySelfer } func renderTime(t time.Time) string { @@ -272,9 +273,7 @@ func (m *Metric) CodecEncodeSelf(encoder *codec.Encoder) { // CodecDecodeSelf implements codec.Selfer func (m *Metric) CodecDecodeSelf(decoder *codec.Decoder) { in := WireMetrics{} - if err := decoder.Decode(&in); err != nil { - return - } + in.CodecDecodeSelf(decoder) *m = in.FromIntermediate() } From a884ceae069d27b29daf453d75486e258a60e776 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 22 Mar 2017 09:51:36 +0000 Subject: [PATCH 3/4] Tests need to depend on code generation --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 35d16c85b..793af8866 100644 --- a/Makefile +++ b/Makefile @@ -122,7 +122,7 @@ $(RUNSVINIT): $(SCOPE_BACKEND_BUILD_UPTODATE) shell: $(SCOPE_BACKEND_BUILD_UPTODATE) /bin/bash -tests: $(SCOPE_BACKEND_BUILD_UPTODATE) +tests: $(SCOPE_BACKEND_BUILD_UPTODATE) $(CODECGEN_TARGETS) ./tools/test -no-go-get lint: $(SCOPE_BACKEND_BUILD_UPTODATE) From 97dda9454d4dae349580e90d7f3341331ce943ee Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 23 Mar 2017 09:53:40 +0000 Subject: [PATCH 4/4] Fix mismatched code coverage errors 'codecgen' embeds a random integer in each identifier; this means code coverage across different CircleCI lanes may not match. Here we force the integer to 23 on every CircleCI build so they always match. --- Makefile | 5 +++-- circle.yml | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 793af8866..50c4f0b1f 100644 --- a/Makefile +++ b/Makefile @@ -18,6 +18,7 @@ DOCKER_DISTRIB_URL=https://get.docker.com/builds/Linux/x86_64/docker-$(DOCKER_VE RUNSVINIT=vendor/runsvinit/runsvinit CODECGEN_DIR=vendor/github.com/ugorji/go/codec/codecgen CODECGEN_EXE=$(CODECGEN_DIR)/bin/codecgen_$(shell go env GOHOSTOS)_$(shell go env GOHOSTARCH) +CODECGEN_UID=0 GET_CODECGEN_DEPS=$(shell find $(1) -maxdepth 1 -type f -name '*.go' -not -name '*_test.go' -not -name '*.codecgen.go' -not -name '*.generated.go') CODECGEN_TARGETS=report/report.codecgen.go render/detailed/detailed.codecgen.go RM=--rm @@ -93,7 +94,7 @@ $(SCOPE_EXE) $(RUNSVINIT) lint tests shell prog/staticui/staticui.go prog/extern --net=host \ -e GOARCH -e GOOS -e CIRCLECI -e CIRCLE_BUILD_NUM -e CIRCLE_NODE_TOTAL \ -e CIRCLE_NODE_INDEX -e COVERDIR -e SLOW -e TESTDIRS \ - $(SCOPE_BACKEND_BUILD_IMAGE) SCOPE_VERSION=$(SCOPE_VERSION) GO_BUILD_INSTALL_DEPS=$(GO_BUILD_INSTALL_DEPS) $@ + $(SCOPE_BACKEND_BUILD_IMAGE) SCOPE_VERSION=$(SCOPE_VERSION) GO_BUILD_INSTALL_DEPS=$(GO_BUILD_INSTALL_DEPS) CODECGEN_UID=$(CODECGEN_UID) $@ else @@ -110,7 +111,7 @@ $(SCOPE_EXE): $(SCOPE_BACKEND_BUILD_UPTODATE) %.codecgen.go: $(CODECGEN_EXE) rm -f $@; $(GO_HOST) build $(GO_BUILD_FLAGS) ./$(@D) # workaround for https://github.com/ugorji/go/issues/145 - cd $(@D) && $(WITH_GO_HOST_ENV) $(shell pwd)/$(CODECGEN_EXE) -rt $(GO_BUILD_TAGS) -u -o $(@F) $(notdir $(call GET_CODECGEN_DEPS,$(@D))) + cd $(@D) && $(WITH_GO_HOST_ENV) $(shell pwd)/$(CODECGEN_EXE) -d $(CODECGEN_UID) -rt $(GO_BUILD_TAGS) -u -o $(@F) $(notdir $(call GET_CODECGEN_DEPS,$(@D))) $(CODECGEN_EXE): $(CODECGEN_DIR)/*.go mkdir -p $(@D) diff --git a/circle.yml b/circle.yml index ce9c031fc..7577c3480 100644 --- a/circle.yml +++ b/circle.yml @@ -42,7 +42,7 @@ test: override: - cd $SRCDIR; make RM= lint: parallel: true - - cd $SRCDIR; COVERDIR=./coverage make RM= tests: + - cd $SRCDIR; COVERDIR=./coverage make RM= CODECGEN_UID=23 tests: parallel: true - cd $SRCDIR; make RM= client-test static: parallel: true