-
Notifications
You must be signed in to change notification settings - Fork 16k
Add .NET 5 target and improve WriteString performance with SIMD #8147
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
Add .NET 5 target and improve WriteString performance with SIMD #8147
Conversation
21f9dd5 to
9189e56
Compare
|
The other writeString optimization PR has been merged, let's rebase? The C# build is currently failing: A new enough version of the runtime needs to be installed in the test scripts or in the dockerfile. |
addd4e2 to
93fafb0
Compare
|
Updated: Before: After: Benchmarks that use the new path are 105 and 10080 length strings. Both are much faster: Before: After: 🔥 |
7c383ba to
fd117d0
Compare
I've attempted to fix this but can't trigger the build to test. If it doesn't work can you finish making the changes required here. |
|
The linux tests for netcoreapp2.1 and net50 on linux are failing: |
|
On Windows I see this error: |
|
Are you sure that is on Windows? The paths in the error message look like its Linux. I think this is the problem: NuGet/Announcements#49 |
ad04542 to
8932446
Compare
|
Fixed test. |
|
I think this fixes the Linux restore issue: - FROM debian:stretch
+ FROM debian:bullseye |
cfea429 to
d9889a4
Compare
jtattermusch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like I've been able to update the CI stuff so that the tests are now passing.
Left a few more additional comments (we're almost there).
jtattermusch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the extra comments.
|
I'll merge once the tests finish. |
|
The C++ distcheck tests failure https://fusion2.corp.google.com/invocations/ec4980f4-92e5-460a-9208-77fbf97eca86/targets/protobuf%2Fgithub%2Fmaster%2Fubuntu%2Fcpp_distcheck%2Fpresubmit/log seems to be unrelated. |
@jtattermusch
Optimization improves writing larger all-ASCII strings.
Before
Process 4 chars at a time
Process 4 chars at a time + SSE2/ARM SIMD