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

Improved chacha8 performance with MSVC #271

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ydipeepo
Copy link

This improves the performance of chacha8 a little with MSVC. (around 23%)
Additional my test code and results with same compiler options:

#include <chrono>
#include <vector>
#include <iostream>

#include "chacha8.h"

int main ()
{
	const uint32_t iteration = 100;
	const uint32_t blocks = 4096 * 128;

	std::vector<uint8_t> key;
	std::vector<uint8_t> iv;
	for (int i = 0; i < 32; ++i) key.push_back ((uint8_t)i);
	for (int i = 0; i < 8; ++i) iv.push_back ((uint8_t)i);

	std::vector<uint8_t> out1;
	std::vector<uint8_t> out2;
	out1.resize (blocks * 64);
	out2.resize (blocks * 64);

	chacha8_ctx ctx;
	chacha8_keysetup (&ctx, key.data (), 256, iv.data ());

	std::vector<double> res;
	for (auto i = 0; i < iteration; ++i)
	{
		auto start_org = std::chrono::steady_clock::now ();
		chacha8_get_keystream (&ctx, 0, blocks, out1.data ());
		auto end_org = std::chrono::steady_clock::now ();

		auto start_new = std::chrono::steady_clock::now ();
		chacha8_get_keystream_cst (&ctx, 0, blocks, out2.data ());
		auto end_new = std::chrono::steady_clock::now ();

		if (std::equal (out1.cbegin (), out1.cend (), out2.cbegin ()))
		{
			const auto r =
				(double)std::chrono::duration_cast<std::chrono::nanoseconds>(end_org - start_org).count () /
				(double)std::chrono::duration_cast<std::chrono::nanoseconds>(end_new - start_new).count ();
			res.push_back (r);
			std::cout << "org/new: " << r << std::endl;
		}
		else
			std::cout << "ERR" << std::endl;
	}

	auto avg = 0.0;
	for (auto& r : res) avg += r;
	std::cout << "org/new mean: " << avg / res.size () << std::endl;
}
... abbrev ...
org/new: 1.23318
org/new: 1.21971
org/new: 1.23604
org/new: 1.23034
org/new: 1.24382
org/new: 1.23477
org/new: 1.22043
org/new: 1.25348
org/new: 1.22831
org/new: 1.222
org/new: 1.25699
org/new: 1.23271
org/new: 1.23033
org/new: 1.2323
org/new: 1.22129
org/new: 1.68185
org/new: 1.25155
org/new: 0.963338
org/new: 1.40123
org/new: 1.2432
org/new mean: 1.23764

It has passed all tests. Thanks for your review.

@ydipeepo ydipeepo closed this Jun 11, 2021
@stotiks
Copy link

stotiks commented Jun 12, 2021

Why closed?

@hoffmang9 hoffmang9 reopened this Jun 12, 2021
@hoffmang9
Copy link
Member

Not sure why - re-opening

@ydipeepo
Copy link
Author

@hoffmang9 @stotiks Just simply embrassed.

@ydipeepo
Copy link
Author

And also added GCC thunk. 4~5% faster than before but only x64:

... abbrev ...
org/new: 1.04859
org/new: 1.04669
org/new: 1.08676
org/new: 1.05594
org/new: 1.05845
org/new: 1.05931
org/new: 1.04588
org/new: 1.04549
org/new: 1.04905
org/new: 1.05218
org/new mean: 1.04996

@stotiks
Copy link

stotiks commented Jun 12, 2021

Create also pull request for https://github.com/madMAx43v3r/chia-plotter

@arvidn
Copy link
Contributor

arvidn commented Aug 3, 2021

I'm not sure we have sufficient test coverage of chacha8 to make changes with confidence. Could we have a CI test to run this against a reference implementation?

@github-actions
Copy link

github-actions bot commented Oct 3, 2021

'This PR has been flagged as stale due to no activity for over 60
days. It will not be automatically closed, but it has been given
a stale-pr label and should be manually reviewed.'

@github-actions
Copy link

github-actions bot commented Dec 3, 2021

'This PR has been flagged as stale due to no activity for over 60
days. It will not be automatically closed, but it has been given
a stale-pr label and should be manually reviewed.'

@github-actions
Copy link

github-actions bot commented Feb 2, 2022

'This PR has been flagged as stale due to no activity for over 60
days. It will not be automatically closed, but it has been given
a stale-pr label and should be manually reviewed.'

@wjblanke
Copy link
Contributor

This looks great but needs some dev time to review

@github-actions github-actions bot removed the stale-pr label Mar 26, 2022
@github-actions
Copy link

'This PR has been flagged as stale due to no activity for over 60
days. It will not be automatically closed, but it has been given
a stale-pr label and should be manually reviewed.'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants