-
Notifications
You must be signed in to change notification settings - Fork 15.6k
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
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
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).
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