muktihari / fit

A FIT SDK for decoding and encoding Garmin FIT files in Go supporting FIT Protocol V2.
BSD 3-Clause "New" or "Revised" License
24 stars 2 forks source link

perf: early validate all messages before encoding #454

Closed muktihari closed 2 weeks ago

muktihari commented 2 weeks ago

Validating messages early give several advantages:

  1. If invalid message occurs, we can return early without doing unnecessary works such as marshaling, calculate CRC or even do a syscall to write to the file.
  2. If normal writer is given (a writer without offset writer capability), the encoding speed is improved by geomean 16% since we don't need to validate messages twices. Benchmark:
    goos: linuxl goarch: amd64l pkg: benchfitl cpu: Intel(R) Core(TM) i5-5257U CPU @ 2.70GHz
                             │   old.txt   │               new.txt               │
                             │   sec/op    │   sec/op     vs base                │
    Encode/muktihari/fit_raw-4   93.57m ± 8%   74.63m ± 3%  -20.24% (p=0.000 n=10)
    Encode/muktihari/fit-4       157.2m ± 7%   136.7m ± 2%  -13.01% (p=0.000 n=10)
    geomean                      121.3m        101.0m       -16.71%
  3. We can release all objects generated by the message validator early, so it can be garbage-collected as soon as we are done validating. The objects are only be generated when it encounters developer data.
codecov-commenter commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.94%. Comparing base (e3d3e16) to head (27b5306). Report is 1 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #454 +/- ## =========================================== - Coverage 100.00% 99.94% -0.06% =========================================== Files 42 42 Lines 3659 3687 +28 =========================================== + Hits 3659 3685 +26 - Misses 0 2 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.