Skip to content

Commit

Permalink
perf: reduce decoder malloc & reuse existing vars (#61)
Browse files Browse the repository at this point in the history
* perf: reduce decoder malloc & reuse existing vars

- decoder: use single backing array instead of creating slice on each r read since the max size is known.
- decoder: implement reuse existing variables/instance rather than creating new one.
- proto: remove Size in FieldBase, we can get the size from basetype.
- profile: clean up fitgen's profile builder.go
- generate: profile and factory

* test: add decoder benchmark
  • Loading branch information
muktihari committed Dec 22, 2023
1 parent fc7ab4a commit 5cc889c
Show file tree
Hide file tree
Showing 13 changed files with 1,446 additions and 2,692 deletions.
4 changes: 3 additions & 1 deletion decoder/accumulator.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ type Accumulator struct {
}

func NewAccumulator() *Accumulator {
return &Accumulator{AccumulatedValues: make([]AccumulatedValue, 0)}
return &Accumulator{} // No need to make AccumulatedValues as it will be created on append anyway.
}

func (a *Accumulator) Collect(mesgNum typedef.MesgNum, destFieldNum byte, value int64) {
Expand Down Expand Up @@ -43,6 +43,8 @@ func (a *Accumulator) Accumulate(mesgNum typedef.MesgNum, destFieldNum byte, val
return value
}

func (a *Accumulator) Reset() { a.AccumulatedValues = a.AccumulatedValues[:0] }

type AccumulatedValue struct {
MesgNum typedef.MesgNum
DestFieldNum byte
Expand Down
15 changes: 15 additions & 0 deletions decoder/accumulator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,18 @@ func TestCollect(t *testing.T) {
})
}
}

func TestReset(t *testing.T) {
accumu := NewAccumulator()
accumu.Collect(mesgnum.Record, fieldnum.RecordSpeed, 1000)

if len(accumu.AccumulatedValues) != 1 {
t.Fatalf("expected AccumulatedValues is 1, got: %d", len(accumu.AccumulatedValues))
}

accumu.Reset()

if len(accumu.AccumulatedValues) != 0 {
t.Fatalf("expected AccumulatedValues is 0 after reset, got: %d", len(accumu.AccumulatedValues))
}
}
54 changes: 32 additions & 22 deletions decoder/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ type Decoder struct {
accumulator *Accumulator
crc16 hash.Hash16

// The maximum n field definition in a mesg is 255 and we need 3 byte per field. 255 * 3 = 765 (+1 cap).
// So we will never exceed 766.
backingArray [(255 * 3) + 1]byte

options *options

decodeHeaderOnce func() error // The func to decode header exactly once, return the error of the first invocation if any. Initialized on New().
Expand Down Expand Up @@ -184,7 +188,6 @@ func New(r io.Reader, opts ...Option) *Decoder {
accumulator: NewAccumulator(),
crc16: crc16.New(crc16.MakeFitTable()),
localMessageDefinitions: [proto.LocalMesgNumMask + 1]*proto.MessageDefinition{},
messages: make([]proto.Message, 0),
mesgListeners: options.mesgListeners,
mesgDefListeners: options.mesgDefListeners,
developerDataIds: make([]*mesgdef.DeveloperDataId, 0),
Expand Down Expand Up @@ -234,25 +237,33 @@ func (d *Decoder) Next() bool {
return true
}

// reset values for the next chained Fit file
d.accumulator = NewAccumulator()
d.reset() // reset values for the next chained Fit file

// err is saved in the func, any exported will call this func anyway.
return d.decodeHeaderOnce() == nil
}

func (d *Decoder) reset() {
for i := range d.localMessageDefinitions {
d.localMessageDefinitions[i] = nil
}

d.accumulator.Reset()
d.crc16.Reset()
d.fileHeader = proto.FileHeader{}
d.localMessageDefinitions = [proto.LocalMesgNumMask + 1]*proto.MessageDefinition{}
d.messages = make([]proto.Message, 0)
if !d.options.broadcastOnly {
d.messages = make([]proto.Message, 0) // Must create new.
}
d.fileId = nil
d.developerDataIds = make([]*mesgdef.DeveloperDataId, 0)
d.fieldDescriptions = make([]*mesgdef.FieldDescription, 0)
d.developerDataIds = d.developerDataIds[:0]
d.fieldDescriptions = d.fieldDescriptions[:0]
d.crc = 0
d.cur = 0
d.timestamp = 0
d.lastTimeOffset = 0
d.sequenceCompleted = false

d.initDecodeHeaderOnce() // reset to enable invocation.

// err is saved in the func, any exported will call this func anyway.
return d.decodeHeaderOnce() == nil
}

// DecodeWithContext wraps Decode to respect context propagation.
Expand Down Expand Up @@ -308,7 +319,7 @@ func (d *Decoder) Decode() (fit *proto.Fit, err error) {
}

func (d *Decoder) decodeHeader() error {
b := make([]byte, 1)
b := d.backingArray[:1]
n, err := io.ReadFull(d.r, b)
d.n += int64(n)
if err != nil {
Expand All @@ -320,7 +331,7 @@ func (d *Decoder) decodeHeader() error {
}

size := b[0]
b = make([]byte, size-1)
b = d.backingArray[1:size]
n, err = io.ReadFull(d.r, b)
d.n += int64(n)
if err != nil {
Expand Down Expand Up @@ -351,7 +362,7 @@ func (d *Decoder) decodeHeader() error {
return nil
}

_, _ = d.crc16.Write(append([]byte{size}, b[:11]...))
_, _ = d.crc16.Write(d.backingArray[:12])
if d.crc16.Sum16() != d.fileHeader.CRC { // check header integrity
return ErrCRCChecksumMismatch
}
Expand Down Expand Up @@ -384,7 +395,7 @@ func (d *Decoder) decodeMessage() error {
}

func (d *Decoder) decodeMessageDefinition(header byte) error {
b := make([]byte, 5)
b := d.backingArray[:5]
if err := d.read(b); err != nil {
return err
}
Expand All @@ -397,7 +408,7 @@ func (d *Decoder) decodeMessageDefinition(header byte) error {
}

n := b[4]
b = make([]byte, n*3) // 3 byte per field
b = d.backingArray[:n*3] // 3 byte per field
if err := d.read(b); err != nil {
return err
}
Expand All @@ -417,7 +428,7 @@ func (d *Decoder) decodeMessageDefinition(header byte) error {
return err
}

b = make([]byte, n*3) // 3 byte per field
b := d.backingArray[:n*3] // 3 byte per field
if err := d.read(b); err != nil {
return err
}
Expand Down Expand Up @@ -517,8 +528,7 @@ func (d *Decoder) decodeFields(mesgDef *proto.MessageDefinition, mesg *proto.Mes

field := d.factory.CreateField(mesgDef.MesgNum, fieldDef.Num)
if field.Name == factory.NameUnknown {
// Assign fieldDef's size and type for unknown field so later we can encode it as per its original value.
field.Size = fieldDef.Size
// Assign fieldDef's type for unknown field so later we can encode it as per its original value.
field.Type = profile.ProfileTypeFromString(fieldDef.BaseType.String())
}

Expand Down Expand Up @@ -628,7 +638,7 @@ func (d *Decoder) decodeDeveloperFields(mesgDef *proto.MessageDefinition, mesg *

if fieldDescription == nil {
// Can't interpret this DeveloperField, no FieldDescription found. Just read acquired bytes and move forward.
if err := d.read(make([]byte, devFieldDef.Size)); err != nil {
if err := d.read(d.backingArray[:devFieldDef.Size]); err != nil {
return fmt.Errorf("no field description found, unable to read acquired bytes: %w", err)
}
continue
Expand Down Expand Up @@ -674,7 +684,7 @@ func (d *Decoder) decodeDeveloperFields(mesgDef *proto.MessageDefinition, mesg *
}

func (d *Decoder) decodeCRC() error {
b := make([]byte, 2)
b := d.backingArray[:2]
n, err := io.ReadFull(d.r, b)
d.n += int64(n)
if err != nil {
Expand All @@ -699,7 +709,7 @@ func (d *Decoder) read(b []byte) error {

// readByte is shorthand for read([1]byte).
func (d *Decoder) readByte() (byte, error) {
b := make([]byte, 1)
b := d.backingArray[:1]
if err := d.read(b); err != nil {
return 0, err
}
Expand All @@ -708,7 +718,7 @@ func (d *Decoder) readByte() (byte, error) {

// readValue reads message value bytes from reader and convert it into its corresponding type.
func (d *Decoder) readValue(size byte, baseType basetype.BaseType, isArray bool, arch byte) (any, error) {
b := make([]byte, size)
b := d.backingArray[:size]
if err := d.read(b); err != nil {
return nil, err
}
Expand Down
52 changes: 49 additions & 3 deletions decoder/decoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,6 @@ func TestNext(t *testing.T) {

// Test Begin
dec := New(r)
prevAccumulator := dec.accumulator

if !dec.Next() {
t.Fatalf("should have next, return false")
Expand All @@ -427,14 +426,17 @@ func TestNext(t *testing.T) {
t.Fatal(err)
}

// Manually add accumulated value
dec.accumulator.Collect(mesgnum.Record, fieldnum.RecordSpeed, 1000)

// Check whether after decode, fields are reset and next sequence is retrieved.

if !dec.Next() {
t.Fatalf("should have next, return false")
}

if prevAccumulator == dec.accumulator {
t.Fatalf("expected new accumulator got same")
if len(dec.accumulator.AccumulatedValues) != 0 {
t.Fatalf("expected accumulator's AccumulatedValues is 0, got: %d", len(dec.accumulator.AccumulatedValues))
}

if dec.crc16.Sum16() != 0 { // not necessary since reset every decode header anyway, but let's just add it
Expand Down Expand Up @@ -1402,3 +1404,47 @@ func TestReadValue(t *testing.T) {
})
}
}

func BenchmarkDecodeMessageData(b *testing.B) {
b.StopTimer()
mesg := factory.CreateMesg(typedef.MesgNumRecord)
mesgDef := proto.CreateMessageDefinition(&mesg)
dec := New(nil, WithIgnoreChecksum(), WithNoComponentExpansion())
dec.localMessageDefinitions[0] = &mesgDef
mesgb, _ := mesg.MarshalBinary()
b.StartTimer()

for i := 0; i < b.N; i++ {
dec.r = bytes.NewBuffer(mesgb)
err := dec.decodeMessageData(0)
if err != nil {
b.Fatal(err)
}
}
}

func BenchmarkDecode(b *testing.B) {
b.StopTimer()
// This is not a typical FIT in term of file size (2.3M) and the messages it contains (200.000 messages)
// But since it's big, it's should be good to benchmark.
f, err := os.Open("../testdata/big_activity.fit")
if err != nil {
panic(err)
}
defer f.Close()

all, err := io.ReadAll(f)
if err != nil {
panic(err)
}
b.StartTimer()

dec := New(nil)
for i := 0; i < b.N; i++ {
dec.r = bytes.NewBuffer(all)
for dec.Next() {
_, _ = dec.Decode()
}
dec.reset()
}
}
3 changes: 0 additions & 3 deletions encoder/encoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"github.com/muktihari/fit/kit/datetime"
"github.com/muktihari/fit/kit/hash/crc16"
"github.com/muktihari/fit/profile"
"github.com/muktihari/fit/profile/basetype"
"github.com/muktihari/fit/profile/typedef"
"github.com/muktihari/fit/profile/untyped/fieldnum"
"github.com/muktihari/fit/profile/untyped/mesgnum"
Expand Down Expand Up @@ -608,7 +607,6 @@ func TestEncodeMessage(t *testing.T) {
{
FieldBase: &proto.FieldBase{
Name: factory.NameUnknown,
Size: basetype.Sint64.Size(),
Type: profile.Sint64, // int64 type is ilegal for protocol v1.0
},
Value: int64(1234),
Expand All @@ -627,7 +625,6 @@ func TestEncodeMessage(t *testing.T) {
{
FieldBase: &proto.FieldBase{
Name: factory.NameUnknown,
Size: basetype.Sint64.Size(),
Type: profile.Sint64, // int64 type is ilegal for protocol v1.0
},
Value: int64(1234),
Expand Down
6 changes: 0 additions & 6 deletions encoder/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
)

var (
ErrSizeZero = errors.New("size is zero")
ErrValueTypeMismatch = errors.New("value type mismatch")
ErrNoFields = errors.New("no fields")
ErrMissingDeveloperDataId = errors.New("missing developer data id")
Expand Down Expand Up @@ -88,11 +87,6 @@ func (v *messageValidator) Validate(mesg *proto.Message) error {
continue
}

if field.Size == 0 {
return fmt.Errorf("size could not be zero for fieldIndex: %d fieldNum: %d, fieldName: %s: %w",
i, field.Num, field.Name, ErrSizeZero)
}

if v.options.omitInvalidValues && !hasValidValue(field.Value) {
mesg.Fields = append(mesg.Fields[:i], mesg.Fields[i+1:]...)
i--
Expand Down
13 changes: 0 additions & 13 deletions encoder/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,19 +121,6 @@ func TestMessageValidatorValidate(t *testing.T) {
},
sizes: []int{1},
},
{
name: "mesg contain field with invalid size",
mesgs: []proto.Message{
factory.CreateMesgOnly(mesgnum.Record).WithFields(
proto.Field{
FieldBase: &proto.FieldBase{
Size: 0,
},
},
),
},
errs: []error{ErrSizeZero},
},
{
name: "mesg contain field with scaled value",
mesgs: []proto.Message{
Expand Down
Loading

0 comments on commit 5cc889c

Please sign in to comment.