Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

chore: reduce copy times for bytes in core-hashing #13519

Merged
merged 6 commits into from
Mar 6, 2023
Merged

chore: reduce copy times for bytes in core-hashing #13519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2b4cece
chore: reduce copy bytes for core-hashing
yjhmelody Mar 3, 2023
58ad855
improve by suggestions and remove unused `xx_into`
yjhmelody Mar 3, 2023
da00eef
chore: replace sha2 crate by `sp_core::hashing` for pallet-alliance
yjhmelody Mar 3, 2023
10e9e0c
fix features
yjhmelody Mar 3, 2023
7c0a91c
use sp-core-hashing directly
yjhmelody Mar 3, 2023
f8bfbbb
add to dev-dep
yjhmelody Mar 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions frame/alliance/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ targets = ["x86_64-unknown-linux-gnu"]

[dependencies]
array-bytes = { version = "4.1", optional = true }
sha2 = { version = "0.10.1", default-features = false, optional = true }
log = { version = "0.4.14", default-features = false }

codec = { package = "parity-scale-codec", version = "3.2.2", default-features = false, features = ["derive"] }
scale-info = { version = "2.0.1", default-features = false, features = ["derive"] }

sp-std = { version = "5.0.0", default-features = false, path = "../../primitives/std" }
sp-core = { version = "7.0.0", default-features = false, path = "../../primitives/core" }
sp-core-hashing = { version = "5.0.0", default-features = false, path = "../../primitives/core/hashing", optional = true }
sp-io = { version = "7.0.0", default-features = false, path = "../../primitives/io" }
sp-runtime = { version = "7.0.0", default-features = false, path = "../../primitives/runtime" }

Expand All @@ -34,13 +34,14 @@ pallet-collective = { version = "4.0.0-dev", path = "../collective", default-fea

[dev-dependencies]
array-bytes = "4.1"
sha2 = "0.10.1"
sp-core-hashing = { version = "5.0.0", default-features = false, path = "../../primitives/core/hashing" }
pallet-balances = { version = "4.0.0-dev", path = "../balances" }
pallet-collective = { version = "4.0.0-dev", path = "../collective" }

[features]
default = ["std"]
std = [
"sp-core-hashing?/std",
"pallet-collective?/std",
"frame-benchmarking?/std",
"log/std",
Expand All @@ -56,7 +57,7 @@ std = [
]
runtime-benchmarks = [
"array-bytes",
"sha2",
"sp-core-hashing",
"frame-benchmarking/runtime-benchmarks",
"sp-runtime/runtime-benchmarks",
"frame-support/runtime-benchmarks",
Expand Down
7 changes: 2 additions & 5 deletions frame/alliance/src/benchmarking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,8 @@ fn assert_last_event<T: Config<I>, I: 'static>(generic_event: <T as Config<I>>::
}

fn cid(input: impl AsRef<[u8]>) -> Cid {
use sha2::{Digest, Sha256};
let mut hasher = Sha256::new();
hasher.update(input);
let result = hasher.finalize();
Cid::new_v0(&*result)
let result = sp_core_hashing::sha2_256(input.as_ref());
Cid::new_v0(result)
}

fn rule(input: impl AsRef<[u8]>) -> Cid {
Expand Down
7 changes: 2 additions & 5 deletions frame/alliance/src/mock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,11 +368,8 @@ pub fn new_bench_ext() -> sp_io::TestExternalities {
}

pub fn test_cid() -> Cid {
use sha2::{Digest, Sha256};
let mut hasher = Sha256::new();
hasher.update(b"hello world");
let result = hasher.finalize();
Cid::new_v0(&*result)
let result = sp_core_hashing::sha2_256(b"hello world");
Cid::new_v0(result)
}

pub fn make_remark_proposal(value: u64) -> (RuntimeCall, u32, H256) {
Expand Down
46 changes: 7 additions & 39 deletions primitives/core/hashing/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,48 +35,22 @@ pub fn blake2_512_into(data: &[u8], dest: &mut [u8; 64]) {

/// Do a Blake2 512-bit hash and return result.
pub fn blake2_512(data: &[u8]) -> [u8; 64] {
let mut r = [0; 64];
blake2_512_into(data, &mut r);
r
}

/// Do a Blake2 256-bit hash and place result in `dest`.
pub fn blake2_256_into(data: &[u8], dest: &mut [u8; 32]) {
type Blake2b256 = blake2::Blake2b<U32>;
dest.copy_from_slice(Blake2b256::digest(data).as_slice());
blake2::Blake2b512::digest(data).into()
}

/// Do a Blake2 256-bit hash and return result.
pub fn blake2_256(data: &[u8]) -> [u8; 32] {
let mut r = [0; 32];
blake2_256_into(data, &mut r);
r
}

/// Do a Blake2 128-bit hash and place result in `dest`.
pub fn blake2_128_into(data: &[u8], dest: &mut [u8; 16]) {
type Blake2b128 = blake2::Blake2b<U16>;
dest.copy_from_slice(Blake2b128::digest(data).as_slice());
blake2::Blake2b::<U32>::digest(data).into()
}

/// Do a Blake2 128-bit hash and return result.
pub fn blake2_128(data: &[u8]) -> [u8; 16] {
let mut r = [0; 16];
blake2_128_into(data, &mut r);
r
}

/// Do a Blake2 64-bit hash and place result in `dest`.
pub fn blake2_64_into(data: &[u8], dest: &mut [u8; 8]) {
type Blake2b64 = blake2::Blake2b<U8>;
Copy link
Member

Choose a reason for hiding this comment

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

Are the *_into(data, dest) family of functions relevant?
Looks like are not used anywere in the codebase and is some user requires something like that it can easily construct it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you means that should I remove these xx_into functions?

Copy link
Member

Choose a reason for hiding this comment

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

I usually tend to follow YAGNI principle.
But with that I'm not saying that should be a dogma.
I have the (maybe bad?!) attitude to remove unused code especially the trivial bloat that can be trivially implemented using a line of code by the user 😃

dest.copy_from_slice(Blake2b64::digest(data).as_slice());
blake2::Blake2b::<U16>::digest(data).into()
}

/// Do a Blake2 64-bit hash and return result.
pub fn blake2_64(data: &[u8]) -> [u8; 8] {
let mut r = [0; 8];
blake2_64_into(data, &mut r);
r
blake2::Blake2b::<U8>::digest(data).into()
}

/// Do a XX 64-bit hash and place result in `dest`.
Expand Down Expand Up @@ -128,21 +102,15 @@ pub fn twox_256(data: &[u8]) -> [u8; 32] {

/// Do a keccak 256-bit hash and return result.
pub fn keccak_256(data: &[u8]) -> [u8; 32] {
let mut output = [0u8; 32];
output.copy_from_slice(sha3::Keccak256::digest(data).as_slice());
output
sha3::Keccak256::digest(data).into()
}

/// Do a keccak 512-bit hash and return result.
pub fn keccak_512(data: &[u8]) -> [u8; 64] {
let mut output = [0u8; 64];
output.copy_from_slice(sha3::Keccak512::digest(data).as_slice());
output
sha3::Keccak512::digest(data).into()
}

/// Do a sha2 256-bit hash and return result.
pub fn sha2_256(data: &[u8]) -> [u8; 32] {
let mut output = [0u8; 32];
output.copy_from_slice(sha2::Sha256::digest(data).as_slice());
output
sha2::Sha256::digest(data).into()
}