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

Add custom MD5/SHA256 hasher option #1283

Merged
merged 4 commits into from
Jun 2, 2020

Conversation

klauspost
Copy link
Contributor

@klauspost klauspost commented May 4, 2020

Since there is no cleanup for the API clients using the SIMD MD5 needs to be up to the API users to manage.

Provide a function that allows to insert a custom hasher for MD5 and SHA256.

We keep using the same for both, but by default we allow to reuse the hashers.

Since there is no cleanup for the API clients using the SIMD MD5 needs to be up to the API users to manage.

Provide a function that allows to insert a custom hasher.

md5-simd package is only used to provide the interface, by default a wrapper for "crypto/md5" is created, so default performance remains unaffected.
klauspost added a commit to klauspost/warp that referenced this pull request May 4, 2020
Bonus changes:
* Compress output a bit better
* Clean up put options
* Move `disable-multipart` to IO options
* Add comment value for CSV input, so we can add stuff like cmdline to output later.

Waiting for minio/minio-go#1283
@fwessels
Copy link
Contributor

fwessels commented May 5, 2020

Code looks good, but need to sort of the md5-simd buffer copy issue first.

@harshavardhana
Copy link
Member

EDIT: Maybe wait until we've found out how to make md5-simd performant.

Do we need it anymore? @klauspost

@klauspost klauspost changed the title Add custom MD5 hasher option Add custom MD5/SHA256 hasher option Jun 2, 2020
@klauspost
Copy link
Contributor Author

@harshavardhana I have kept the default to be a wrapped stdlib, but added a wrapper that will allow reuse of the hashers.

I think this will be a fine improvement and it gives the flexibility to override them on clients.

@harshavardhana
Copy link
Member

PTAL @fwessels

Copy link
Contributor

@fwessels fwessels left a comment

Choose a reason for hiding this comment

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

LGTM

@harshavardhana harshavardhana merged commit bc3a8c5 into minio:master Jun 2, 2020
@klauspost klauspost deleted the add-custom-md5-hasher-option branch June 3, 2020 09:01
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.

3 participants