pion / sdp

A Go implementation of the SDP
https://pion.ly/
MIT License
148 stars 56 forks source link

rewrite marshal/unmarshal #170

Closed paulwe closed 7 months ago

paulwe commented 7 months ago

improves marshal/unmarshal performance and reduces the number of heap objects held by SessionDescription structs. this should be backward compatible except for all the places where pointers are replaced with value types...

BenchmarkMarshal
BenchmarkMarshal-10           659898          1778 ns/op        2065 B/op         53 allocs/op
BenchmarkMarshal-10           682086          1807 ns/op        2065 B/op         53 allocs/op
BenchmarkMarshal-10           638420          1780 ns/op        2065 B/op         53 allocs/op
BenchmarkMarshal-10           664224          1782 ns/op        2065 B/op         53 allocs/op
BenchmarkMarshal-10           678384          1762 ns/op        2065 B/op         53 allocs/op
BenchmarkUnmarshal
BenchmarkUnmarshal-10         240928          4982 ns/op        2576 B/op        108 allocs/op
BenchmarkUnmarshal-10         239852          5000 ns/op        2576 B/op        108 allocs/op
BenchmarkUnmarshal-10         237987          5000 ns/op        2576 B/op        108 allocs/op
BenchmarkUnmarshal-10         238323          5299 ns/op        2576 B/op        108 allocs/op
BenchmarkUnmarshal-10         238104          4999 ns/op        2576 B/op        108 allocs/op
BenchmarkMarshal
BenchmarkMarshal-10          2057742           624.6 ns/op       640 B/op          1 allocs/op
BenchmarkMarshal-10          2110321           571.7 ns/op       640 B/op          1 allocs/op
BenchmarkMarshal-10          2088423           575.0 ns/op       640 B/op          1 allocs/op
BenchmarkMarshal-10          2079690           573.8 ns/op       640 B/op          1 allocs/op
BenchmarkMarshal-10          2035360           576.8 ns/op       640 B/op          1 allocs/op
BenchmarkUnmarshal
BenchmarkUnmarshal-10        1000000          1057 ns/op         920 B/op         14 allocs/op
BenchmarkUnmarshal-10        1000000          1048 ns/op         920 B/op         14 allocs/op
BenchmarkUnmarshal-10        1000000          1058 ns/op         920 B/op         14 allocs/op
BenchmarkUnmarshal-10        1000000          1061 ns/op         920 B/op         14 allocs/op
BenchmarkUnmarshal-10        1000000          1053 ns/op         920 B/op         14 allocs/op
codecov[bot] commented 7 months ago

Codecov Report

Attention: Patch coverage is 74.13442% with 254 lines in your changes are missing coverage. Please review.

Project coverage is 65.84%. Comparing base (fb77fb3) to head (d8d785d).

Files Patch % Lines
unmarshal.go 66.37% 118 Missing and 38 partials :warning:
util.go 34.84% 40 Missing and 3 partials :warning:
marshal.go 84.29% 30 Missing :warning:
jsep.go 10.00% 9 Missing :warning:
media_description.go 84.61% 4 Missing and 2 partials :warning:
extmap.go 91.89% 3 Missing :warning:
session_description.go 95.71% 3 Missing :warning:
common_description.go 97.50% 2 Missing :warning:
time_description.go 92.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #170 +/- ## ========================================== + Coverage 62.25% 65.84% +3.58% ========================================== Files 11 10 -1 Lines 1346 1414 +68 ========================================== + Hits 838 931 +93 + Misses 409 403 -6 + Partials 99 80 -19 ``` | [Flag](https://app.codecov.io/gh/pion/sdp/pull/170/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | | |---|---|---| | [go](https://app.codecov.io/gh/pion/sdp/pull/170/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | `?` | | | [wasm](https://app.codecov.io/gh/pion/sdp/pull/170/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | `65.84% <74.13%> (+3.58%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#carryforward-flags-in-the-pull-request-comment) to find out more.

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

Sean-Der commented 7 months ago

@paulwe I bumped the go.mod to start a /v4 release for the breaking changes

Sean-Der commented 7 months ago

@paulwe It would be nice to break this commit up into what has changed.

Others will come and read when a breaking change happened (so they can fix their code). It would be nice if we could give them a log that is useful!

paulwe commented 7 months ago

i can expand on what has changed in the commit message but i'm definitely not going through this and splitting it up on the off chance someone is using this library but can't figure out how to change pointers to value types...