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

SHA1 and SHA256 acceleration #2090

Closed
3 of 4 tasks
noloader opened this issue Nov 21, 2022 · 5 comments
Closed
3 of 4 tasks

SHA1 and SHA256 acceleration #2090

noloader opened this issue Nov 21, 2022 · 5 comments

Comments

@noloader
Copy link
Contributor

noloader commented Nov 21, 2022

Checklist

  • I looked at https://github.com/pbatard/rufus/wiki/FAQ to see if my question has already been answered.
  • I performed a search in the issue tracker for similar issues using keywords relevant to my problem, such as the error message I got from the log.
  • I clicked the 'Log' button or pressed Ctrl-L in Rufus, and copy/pasted the log into the line that says <FULL LOG> below.
  • The log I am copying is the FULL log, starting with the line Rufus version: x.y.z - I have NOT removed any part of it.

I noticed checksum.c is used to calculate hashes of files. I have some experience in implementing crypto operations using compiler intrinsics. See, for example, here, here and here.

My question is, would RUFUS be interested in adding acceleration for SHA-1 and SHA-256? It will be the same stuff Botan and Crypto++ use via intrinsics.

If RUFUS is interested in GCC support, then Function Multiversioning is an easy way to have the alternate routines wired-up. The current C implementation would use __attribute__ ((target ("default"))), and the SHA transforms would use __attribute__ ((target ("sha"))) to enable the ISA. The program will automagically switch to the proper implementation at runtime.

The nice thing about intrinsics is, they work under nearly all operating systems, including Linux, MinGW and Windows. Its write once, run anywhere.

@pbatard
Copy link
Owner

pbatard commented Nov 21, 2022

would RUFUS be interested in adding acceleration for SHA-1 and SHA-256?

We would. Thanks a lot if you want to volunteer to look into this!

I need to point out however that this will be dependent on how much overhead this would add. The reason is that Rufus is a "can also compute SHA" utility rather than "designed first and foremost to compute SHA" (otherwise, we wouldn't limit SHA computation to images only), and as much as I'd like to she speed of checksum computation improved, I don't want to add complexity that would make the application harder to maintain, such as multiplying sources (like Crypto++ does for AVX, SSE) or forcing/multiplying binaries that only work on hardware that supports for specific extensions (such as forcing ARMv8). In short, unlike other applications, we are not interested in being the fastest at any cost. Instead we're going to prefer improvements that don't require heavy code change in checksum.c if achievable.

Oh, and since you're only mentioning SHA-1 and SHA-256, I'm hoping that you can propose improvements for SHA-512 as well.

If RUFUS is interested in GCC support

We will only accept a proposal that supports both MSVC and gcc, since we compile for both (and produce releases for both, with the regular release being MinGW and the Windows Store release being MSVC).

One thing I should mention however, before you decide if you want to start working on these improvements, is that the vast majority of people are going to be running a 32-bit version of x86 Rufus, even on 64-bit hardware, because the download we have on our site point to 32-bit (this is done to avoid the "What version should I download?" questions as well as reduce our maintenance overhead). So part of the improvements you have in mind are likely going to be lost on users on account of that, as expected and as my tests show, there is a noticeable speed degradation when using 32-bit compared to 64-bit, which we don't care that much about right now to want to release a separate 64-bit version. Ultimately, we plan to switch to release a single 64-bit only binary, just like Windows 11 dropped x86_32 altogether, but it will probably be another couple of years before we do that. So you may want to take into consideration that, for most users, the speed improvements you propose may not be as effective as they could be, as they'll be using an x86_32 binary anyway...

So, while I am interested, I'll leave you decide if you think it's worth your time to try to work on these improvements within our specific constraints... At any rate, you do have my thanks for volunteering to see what you can do to help our codebase.

@noloader
Copy link
Contributor Author

noloader commented Nov 24, 2022

Thanks @pbatard,

Let's kick some code around and see if its something you want in rufus.

I added you to my clone as a Collaborator. I find it is the most expedient way to collaborate on code. We can both make changes without the need for a bunch of emails.

@pbatard
Copy link
Owner

pbatard commented Nov 24, 2022

Hmm, if you expect this to be a collaboration on adding code, then I'm afraid I'm going to have to dash that idea.

Improving SHA is not high priority for me enough to allocate any time besides reviewing patch proposals. In other words, I have no intention to deallocate time from all the other features and projects I have lined up, and that are currently starving for time, in order to to make this a collaborative effort. To put it more simply: I'm happy enough with SHA computation as it stands not to want to allocate my time improving it right now.

So, really, I'm pretty much expecting you to submit what you think is a working PR for me to review, as anything more than this is not something I'm going to be able to budget, time-wise, at the moment.

Maybe this is different from other projects, that have enough manpower for one or two developers to take time aside in order to help shaping a PR, through a collaborative effort. But I'm afraid that, currently, this is just not feasible for Rufus: I have just way too many things lined up, and SHA improvements are simply not high enough to want to reshape my priority stack.

So, once again, I will leave it up to you to decide whether the specifics of Rufus that have exposed above make you want to spend time crafting a PR on your own, so that I can review it (NB: I guess I'd be fine with tentative PRs if you want to get some feedback as you are working on it) or if that sounds like too much of a hassle.

@pbatard
Copy link
Owner

pbatard commented Dec 8, 2022

Closed in 36f4716.

@pbatard pbatard closed this as completed Dec 8, 2022
@github-actions
Copy link

github-actions bot commented Mar 9, 2023

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue if you think you have a related problem or query.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants