-
Notifications
You must be signed in to change notification settings - Fork 442
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Performance and bugfixes used by Borg #1192
base: master
Are you sure you want to change the base?
Conversation
workaround for sipsorcery-org#1036
for some reason .1 did not include the changes o-O
… them efficiently
…en underlying transport is not connected
👀 This looks really dope! I was JUST looking into this library for using its implementation of datachannels! Hopefully this will make this library as performant as something like Pion or lib-datachannels |
Sadly not there. I peak at 30MB/s while Pion is able to do full 1Gbps (e.g. ~100MB/s). Funnily enough, Rust implementation has the same problem. I don't believe we are hitting the performance ceiling of .NET here. I think the flow control implementation is incorrect. |
Yeah, i mean Go and C# are about equivalent performance wise (I wouldn't be surprised if .NET is actually faster). Maybe there's some cool stuff Go is doing with Goroutines? |
I guess it's time this library branches into specific .net versions(?) I think that's also what's been holding back the performance(?) Which part would not be implemented if you still supported those versions? |
There's really no part of the library that would not work on these targets. But because they are out of support having them is mostly waste of resources. |
Unless there's a significant performance hit when having legacy support, |
Thanks for the PR. It's a big chunk of work! I used the Borg.SIPSorcery nuget package for the client side of the tests here and got 4 failures with the servers below:
aiortc also fails but that's currently happening with the main SIPSorcery v8.0.0 package due to ECDSA ciphersuites only being offered in the DTLS handshake for some reason. I suspect the failures are mostly related to the BouncyCastle update. Every other time that has been attempted DTLS breaks. |
@sipsorcery can you help me build one of these? I'm on Windows, not sure where to do Or tell me if something is easier to build starting off just Visual Studio 2022. |
I wouldn't recommend attempting to build them manually unless you have a reason to do so (and note some, like libwebrtc, can take weeks to succeed). Instead you can jsut use the prebuilt docker images. They're hosted in a public container registry. For libdatachannel just run:
If it works you'll have the a WebRTC peer sitting there waiting for a connection. Easiest way to test is then to just use dotnet run in the sipsorcery client directory. |
@sipsorcery should probably mention prebuilt container in the README of echoes |
It is... Always has been. |
@sipsorcery damn I am sorry, bad day ) |
@ris-work dang, thanks for notifying. |
Uses BouncyCastle 2
Removes a good chunk of allocations and uses
Span<byte>
instead.Parsed packets are no longer allocated either. Instead, they refer to a preallocated slice of some buffer, and their properties simply read data at corresponding offsets.
Rewrote logging in a few locations to use logging template strings (using C# string interpolation causes allocations and should never be used in critical performance paths).
Many other small performance fixes.
Some things to worry about
Drops support for .NET Core 3.1 and .NET 5.0 because they are no longer supported by MS. (could be reverted, but why?)
Also drops .NET Standard 2.1 because it could only really be used by the above two.
I had to hack datachannel flow control because the implementation SIPSorcery had would eventually slow to a crawl I assume due to a bug somewhere. I could not find that somewhere, so instead made some changes to flow control as seemed more appropriate. I remember doing fixes to something that I believe was not implemented according to the standard, but I believe I also made changes that have nothing to do with the standard.
Fixes
#1088
#1070
#1173
a few other data race issues due to lack of proper synchronization
maybe see the commit titles to get some more ideas
P.S. Potentially supersedes #1190
P.P.S. I made this available as https://www.nuget.org/packages/Borg.SIPSorcery/