Make header and URI parameters ordered#282
Conversation
|
Hi @dennwc . Nice once more. I had probably somewhere very exact change, but I can not recall why I didn't push it. I guess I didn't saw some improvements. I was annoyed for no order as well. I will have to check, but I believe benchmarks should show some improvements. Generally I know this makes more sense due to low number of key values. I only see no improvement or need to change/deprecate current API. Please check comments. Happy to merge this for next BIG release. |
|
@emiago would you be open to merging this into a separate branch of sipgo, so that we can use it |
|
@dennwc You thumbed up my changes request. I think they are small? I just do not find time now todo it myself, but I will merge once they are there? |
|
Done! Should be good now. |
| "github.com/arl/statsviz" | ||
| "github.com/emiago/sipgo/sip" | ||
|
|
||
| _ "net/http/pprof" |
There was a problem hiding this comment.
Can you not remove this. I used this example for heavy loads and profiling
|
@dennwc merged! |
This changes switches from a map representation of header/URI params to a slice.
What changes:
HeaderParamsis now safe to use, it doesn't requireNewParams().Benchmarking results
~25-30% faster header parsing, ~50-60% less allocations when parsing headers with params.
The
HeaderParams/MAPshows the same performance improvement, but a bit more allocations. I believe this is caused byNewParams()allocating a bit more space when necessary to compensate for the real-world usage.Typically, the number of parameters is limited by 4-5, which doesn't justify the overhead of Go maps, so these numbers make sense for the real SIP headers.
Compatibility with existing code
Since the underlying type of
HeaderParamsis different, there are a couple of changes to the client code that must be done:NewParams,Get,Add), then no changes are necessary!{"a":"1", "b":"2"}to{{"a","1"}, {"b":"2"}}.params[k]must be replaced withparams.Get(k)orparams.GetOr(k, "")(second one omits bool return).There are some side affects that must be considered:
HeaderParamsis no longer a reference type, so passing it to functions and expecting changes to propagate requires an extra pointer.AddandRemovenow require a pointer receiver. This should not require changes, since map was always a reference.