refactor: simplified proto marshal-unmarshal related items (#193)

README.md

@@ -326,19 +326,19 @@ goos: darwin
 goarch: amd64
 pkg: benchfit
 cpu: Intel(R) Core(TM) i5-5257U CPU @ 2.70GHz
-BenchmarkDecode/muktihari/fit_raw-4  10   110165214 ns/op   77077057 B/op    100047 allocs/op
-BenchmarkDecode/muktihari/fit-4       9   121938626 ns/op   97035515 B/op    200066 allocs/op
-BenchmarkDecode/tormoder/fit-4        9   112157057 ns/op   84108961 B/op    700051 allocs/op
-BenchmarkEncode/muktihari/fit_raw-4  13    87579657 ns/op      12444 B/op        16 allocs/op
-BenchmarkEncode/muktihari/fit-4       7   151469391 ns/op   44065838 B/op    100021 allocs/op
-BenchmarkEncode/tormoder/fit-4        1  1300131309 ns/op  101992736 B/op  12100314 allocs/op
+BenchmarkDecode/muktihari/fit_raw-4  12    95655408 ns/op   77092784 B/op    100047 allocs/op
+BenchmarkDecode/muktihari/fit-4      13    87700988 ns/op   52699547 B/op    101064 allocs/op
+BenchmarkDecode/tormoder/fit-4       10   106331934 ns/op   84108932 B/op    700051 allocs/op
+BenchmarkEncode/muktihari/fit_raw-4  15    75523944 ns/op     131522 B/op        14 allocs/op
+BenchmarkEncode/muktihari/fit-4       8   147434340 ns/op   44139516 B/op    100018 allocs/op
+BenchmarkEncode/tormoder/fit-4        1  1301732705 ns/op  101992544 B/op  12100312 allocs/op
 PASS
-ok  	benchfit	10.811s
+ok      benchfit        10.958s
 ```
 
 NOTE: The `1st` on the list, "raw", means we decode the file into the original FIT protocol message structure (similar to the Official FIT SDK implementation in other languages). While the `2nd` decodes messages to **Activity File** struct, which should be equivalent to what the `3rd` does.
 
-The time spent is more or less the same for decoding, but we allocate way fewer objects on the heap for both decoding and encoding. We achieve significantly faster encoding.
+We decode slightly faster and encode significantly faster. We allocate far fewer objects on the heap and have a smaller memory footprint for both decoding and encoding.
 
 ## Contributing
 

decoder/decoder.go

@@ -14,7 +14,6 @@ import (
 	"sync"
 
 	"github.com/muktihari/fit/factory"
-	"github.com/muktihari/fit/kit/byteorder"
 	"github.com/muktihari/fit/kit/hash"
 	"github.com/muktihari/fit/kit/hash/crc16"
 	"github.com/muktihari/fit/kit/scaleoffset"
@@ -38,6 +37,8 @@ var (
 	ErrByteSizeMismatch       = errors.New("byte size mismath")
 )
 
+const littleEndian = 0
+
 // Decoder is FIT file decoder. See New() for details.
 type Decoder struct {
 	readBuffer  *readBuffer // read from io.Reader with buffer without extra copying.
@@ -552,7 +553,11 @@ func (d *Decoder) decodeMessageDefinition(header byte) error {
 	mesgDef.Header = header
 	mesgDef.Reserved = b[0]
 	mesgDef.Architecture = b[1]
-	mesgDef.MesgNum = typedef.MesgNum(byteorder.Select(b[1]).Uint16(b[2:4]))
+	if mesgDef.Architecture == littleEndian {
+		mesgDef.MesgNum = typedef.MesgNum(binary.LittleEndian.Uint16(b[2:4]))
+	} else {
+		mesgDef.MesgNum = typedef.MesgNum(binary.BigEndian.Uint16(b[2:4]))
+	}
 
 	n := int(b[4])
 	b, err = d.readN(n * 3) // 3 byte per field
@@ -927,7 +932,7 @@ func (d *Decoder) readValue(size byte, baseType basetype.BaseType, isArray bool,
 	if err != nil {
 		return val, err
 	}
-	return proto.Unmarshal(b, byteorder.Select(arch), baseType, isArray)
+	return proto.UnmarshalValue(b, arch, baseType, isArray)
 }
 
 // log logs only if logWriter is not nil.

decoder/raw_test.go

@@ -8,12 +8,10 @@ import (
 	"bytes"
 	"encoding/binary"
 	"errors"
-	"fmt"
 	"io"
 	"os"
 	"testing"
 	"time"
-	"unsafe"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/muktihari/fit/factory"
@@ -500,7 +498,6 @@ func BenchmarkRawDecoderDecode(b *testing.B) {
 
 	buf := bytes.NewBuffer(all)
 	dec := NewRaw()
-	fmt.Println(unsafe.Sizeof(*dec))
 	b.StartTimer()
 	for i := 0; i < b.N; i++ {
 		buf.Reset()

encoder/encoder.go

@@ -5,7 +5,6 @@
 package encoder
 
 import (
-	"bytes"
 	"context"
 	"encoding/binary"
 	"errors"
@@ -61,12 +60,9 @@ type Encoder struct {
 	lastHeaderPos int64       // The byte position of the last header.
 	crc16         hash.Hash16 // Calculate the CRC-16 checksum for ensuring header and message integrity.
 
-	// A wrapper that act as a multi writer for writing message definitions and messages to w and crc16 simultaneously.
-	// We don't use io.MultiWriter since we need to change w on every encode message.
-	wrapWriterAndCrc16 *wrapWriterAndCrc16
-
 	options           *options         // Encoder's options.
 	protocolValidator *proto.Validator // Validates message's properties should match the targeted protocol version requirements.
+	localMesgNumLRU   *lru             // LRU cache for writing local message definition
 
 	dataSize uint32 // Data size of messages in bytes for a single FIT file.
 
@@ -75,12 +71,8 @@ type Encoder struct {
 	// and will change every RolloverEvent occurrence.
 	timestampReference uint32
 
-	localMesgNumLRU *lru // LRU cache for writing local message definition
-
-	mesgDef *proto.MessageDefinition // Temporary message definition to reduce alloc.
-	buf     *bytes.Buffer            // Temporary bytes buffer to reduce alloc.
-
-	bytesArray [proto.MaxBytesPerMessageDefinition]byte // Underlying array for buf as well as general purpose array for encoding process.
+	mesgDef    *proto.MessageDefinition       // Temporary message definition to reduce alloc.
+	bytesArray [proto.MaxBytesPerMessage]byte // General purpose array for encoding process.
 
 	defaultFileHeader proto.FileHeader // Default header to encode when not specified.
 }
@@ -206,10 +198,8 @@ func New(w io.Writer, opts ...Option) *Encoder {
 			DataType:        proto.DataTypeFIT,
 			CRC:             0, // calculated during encoding
 		},
-		mesgDef:            &proto.MessageDefinition{},
-		wrapWriterAndCrc16: &wrapWriterAndCrc16{writer: w, crc16: crc16},
+		mesgDef: &proto.MessageDefinition{},
 	}
-	e.buf = bytes.NewBuffer(e.bytesArray[:])
 
 	return e
 }
@@ -285,9 +275,7 @@ func (e *Encoder) updateHeader(header *proto.FileHeader) error {
 
 	header.DataSize = e.dataSize
 
-	e.buf.Reset()
-	_, _ = header.WriteTo(e.buf)
-	b := e.buf.Bytes()
+	b, _ := header.MarshalAppend(e.bytesArray[:0])
 
 	if header.Size >= 14 {
 		_, _ = e.crc16.Write(b[:12]) // recalculate CRC Checksum since header is changed.
@@ -346,9 +334,7 @@ func (e *Encoder) encodeHeader(header *proto.FileHeader) error {
 	}
 	header.ProtocolVersion = byte(e.options.protocolVersion)
 
-	e.buf.Reset()
-	_, _ = header.WriteTo(e.buf)
-	b := e.buf.Bytes()
+	b, _ := header.MarshalAppend(e.bytesArray[:0])
 
 	if header.Size < 14 {
 		n, err := e.w.Write(b[:header.Size])
@@ -406,30 +392,31 @@ func (e *Encoder) encodeMessage(w io.Writer, mesg *proto.Message) error {
 		return err
 	}
 
-	e.buf.Reset()
-	_, _ = e.mesgDef.WriteTo(e.buf)
-	mesgDefBytes := e.buf.Bytes()
-	localMesgNum, isNewMesgDef := e.localMesgNumLRU.Put(mesgDefBytes)            // This might alloc memory since we need to copy the item.
-	mesgDefBytes[0] = (mesgDefBytes[0] &^ proto.LocalMesgNumMask) | localMesgNum // Update the message definition header.
+	b, _ := e.mesgDef.MarshalAppend(e.bytesArray[:0])
+	localMesgNum, isNewMesgDef := e.localMesgNumLRU.Put(b) // This might alloc memory since we need to copy the item.
+	b[0] = (b[0] &^ proto.LocalMesgNumMask) | localMesgNum // Update the message definition header.
 	mesg.Header = (mesg.Header &^ proto.LocalMesgNumMask) | localMesgNum
 
-	e.wrapWriterAndCrc16.writer = w // Change writer
-
 	if isNewMesgDef {
-		n, err := e.wrapWriterAndCrc16.Write(mesgDefBytes)
-		e.dataSize += uint32(n)
-		e.n += int64(n)
+		n, err := w.Write(b)
+		e.n, e.dataSize = e.n+int64(n), e.dataSize+uint32(n)
 		if err != nil {
 			return fmt.Errorf("write message definition failed: %w", err)
 		}
+		_, _ = e.crc16.Write(b)
 	}
 
-	n, err := mesg.WriteTo(e.wrapWriterAndCrc16)
-	e.dataSize += uint32(n)
-	e.n += int64(n)
+	b, err := mesg.MarshalAppend(e.bytesArray[:0])
+	if err != nil {
+		return fmt.Errorf("marshal mesg failed: %w", err)
+	}
+
+	n, err := w.Write(b)
+	e.n, e.dataSize = e.n+int64(n), e.dataSize+uint32(n)
 	if err != nil {
 		return fmt.Errorf("write message failed: %w", err)
 	}
+	_, _ = e.crc16.Write(b)
 
 	return nil
 }
@@ -464,20 +451,6 @@ func (e *Encoder) compressTimestampIntoHeader(mesg *proto.Message) {
 	mesg.RemoveFieldByNum(proto.FieldNumTimestamp)
 }
 
-type wrapWriterAndCrc16 struct {
-	writer io.Writer
-	crc16  hash.Hash16
-}
-
-func (w *wrapWriterAndCrc16) Write(p []byte) (n int, err error) {
-	n, err = w.writer.Write(p)
-	if err != nil {
-		return
-	}
-	_, _ = w.crc16.Write(p)
-	return
-}
-
 func (e *Encoder) encodeCRC() error {
 	b := e.bytesArray[:2]
 	binary.LittleEndian.PutUint16(b, e.crc16.Sum16())
@@ -523,7 +496,6 @@ func (e *Encoder) Reset(w io.Writer, opts ...Option) {
 
 	e.protocolValidator.SetProtocolVersion(e.options.protocolVersion)
 	e.defaultFileHeader.ProtocolVersion = byte(e.options.protocolVersion)
-	e.wrapWriterAndCrc16.writer = w
 }
 
 // EncodeWithContext is similar to Encode but with respect to context propagation.

internal/cmd/benchfit/benchfit_test.go

@@ -34,8 +34,7 @@ func BenchmarkDecode(b *testing.B) {
 		b.StartTimer()
 
 		for i := 0; i < b.N; i++ {
-			// NOTE: We wrap it with *bufio.Reader since tormoder's fit is already implementing similar concept under the hood while we don't.
-			dec := decoder.New(bufio.NewReader(bytes.NewReader(f)))
+			dec := decoder.New(bytes.NewReader(f))
 			_, err = dec.Decode()
 			if err != nil {
 				b.Fatalf("decode error: %v", err)
@@ -52,7 +51,7 @@ func BenchmarkDecode(b *testing.B) {
 
 		for i := 0; i < b.N; i++ {
 			al := filedef.NewListener()
-			dec := decoder.New(bufio.NewReader(bytes.NewReader(f)),
+			dec := decoder.New(bytes.NewReader(f),
 				decoder.WithMesgListener(al),
 				decoder.WithBroadcastOnly(),
 			)