Skip to content
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

Remove our base64 implementation in favour of base64-simd #1938

Merged
merged 13 commits into from
Nov 9, 2022
Merged

Conversation

jjant
Copy link
Contributor

@jjant jjant commented Nov 3, 2022

Motivation and Context

Fixes (#1277), that is, the following protocol tests now pass:

RestJsonBodyMalformedBlobInvalidBase64_case1
RestJsonBodyMalformedBlobInvalidBase64_case2
RestJsonHeaderMalformedStringInvalidBase64MediaType_case1

Description

  • base64-simd requires Rust 1.62.1 to build, so I had to upgrade it here.

Performance

base64-simd is around 30% to 500% faster than our previous hand-rolled implementation (on my M1 MBP at least).

image

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@smithy-lang smithy-lang deleted a comment from github-actions bot Nov 3, 2022
@github-actions
Copy link

github-actions bot commented Nov 3, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

@jjant jjant marked this pull request as ready for review November 3, 2022 13:54
@jjant jjant requested review from a team as code owners November 3, 2022 13:54
@github-actions
Copy link

github-actions bot commented Nov 3, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

Copy link
Collaborator

@rcoh rcoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, in the future please split out MSRV upgrade PRs in case they need to get reverted

@github-actions
Copy link

github-actions bot commented Nov 3, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

@jjant jjant enabled auto-merge (squash) November 4, 2022 10:22
@github-actions
Copy link

github-actions bot commented Nov 4, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

@github-actions
Copy link

github-actions bot commented Nov 9, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

@jjant jjant merged commit 1844660 into main Nov 9, 2022
@jjant jjant deleted the jjant/fix-tests branch November 9, 2022 12:08
@david-perez david-perez mentioned this pull request Nov 18, 2022
2 tasks
@Nugine Nugine mentioned this pull request Jan 18, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants