PoH Performance degradation with rust 1.87+

[roryharr]

Problem

PoH performance degraded when upgrading from 1.86 to 1.87 compiler. This has blocked all future rust upgrades.

The disassembled code is fairly different between 1.86 and current nightly, so I can't comment on every instruction, but the biggest thing that jumped out to me in solana_sha256_hasher::hashv:
In the 1.86 version, there is only one dynamic call out:

_DYNAMIC+0x8a. This is sha2::sha256::compress256

However, in the newer versions there are two dynamic call outs:

_DYNAMIC+0x618: This is sha2::sha256::compress256
_DYNAMIC+0x620: This is solana_sha256_hasher::Hasher::result

Interesting! it appears that solana_sha256_hasher::Hasher::result is inlined in version 1.86, and it is no longer getting inlined in 1.87 and beyond.

I modified the library to add force inline to all function calls (Note to people following chat: Originally i tried this and it didn't work, but that was because I missed a function. After forcing all functions in the implementation it worked correctly).

1.86 without force inline

test bench_poh_hash                   ... bench:   1,397,297.50 ns/iter (+/- 4,870.21)

1.86 with force inline

test bench_poh_hash                   ... bench:   1,391,815.20 ns/iter (+/- 2,922.94)

nightly without force inline

test bench_poh_hash                   ... bench:   1,474,467.60 ns/iter (+/- 6,121.96)

nightly with force inline

test bench_poh_hash                   ... bench:   1,348,648.70 ns/iter (+/- 2,663.92)

Proposed Solution

Add force inline to all functions in impl Hasher in solana-sha256-hasher

Below shows performance with nightly
Before the restart, nightly without the fix is running.
After the restart, nightly with the fix is running.
It is also better than 1.86 (which is not shown on the graph)

[roryharr]

@t-nelson mentioned not depending on sha256-hasher at all. In this test, I removed the dependency on sha256-hasher from poh.rs and just used Sha256 directly. There is no support for target_os = "solana", but I am not sure if that is important in this path as I am unfamiliar with it.

Data is below: It increases performance by another 10% or so

[roryharr]

If this is blocking the rust upgrade though, I think it would be a better approach to do the quick fix for solana-sha256-hasher right now and then worry about optimization.

[t-nelson]

If this is blocking the rust upgrade though, I think it would be a better approach to do the quick fix for solana-sha256-hasher right now and then worry about optimization.

the "quick fix" is the replicode inline, right?

i wonder if there are other places we're going to be bitten by this codegen change. the merkle tree implementation(s(?)) come to mind

[steviez]

Add force inline to all functions in impl Hasher in solana-sha256-hasher

Same as Trent - can you confirm this is the diff ?

diff --git a/sha256-hasher/src/lib.rs b/sha256-hasher/src/lib.rs
index e8785730..39261a8f 100644
--- a/sha256-hasher/src/lib.rs
+++ b/sha256-hasher/src/lib.rs
@@ -11,14 +11,17 @@ pub struct Hasher {
 
 #[cfg(all(feature = "sha2", not(target_os = "solana")))]
 impl Hasher {
+    #[inline(always)]
     pub fn hash(&mut self, val: &[u8]) {
         self.hasher.update(val);
     }
+    #[inline(always)]
     pub fn hashv(&mut self, vals: &[&[u8]]) {
         for val in vals {
             self.hash(val);
         }
     }
+    #[inline(always)]
     pub fn result(self) -> Hash {
         let bytes: [u8; solana_hash::HASH_BYTES] = self.hasher.finalize().into();
         bytes.into()

As for direction, yeah, I agree that making the above tweak is probably an easier path to unblock Rust upgrades. We can refactor that crate out in the future

i wonder if there are other places we're going to be bitten by this codegen change

Yeah, this is a fair concern. This is the only one that was blatant, but we could have slipped in other places. Given that we just branched v3.1, we would have a while to let this soak from master-to-MNB. Could be extra diligent in reviewing metrics now that we know what we're looking for

[t-nelson]

no the attribute was confirmed ineffective on discord. gotta copypasta the code

[steviez]

no the attribute was confirmed ineffective on discord. gotta copypasta the code

From Rory's writeup above:

I modified the library to add force inline to all function calls (Note to people following chat: Originally i tried this and it didn't work, but that was because I missed a function. After forcing all functions in the implementation it worked correctly).

[t-nelson]

[steviez]

[roryharr]

Yes,

diff --git a/sha256-hasher/src/lib.rs b/sha256-hasher/src/lib.rs
index e8785730..39261a8f 100644
--- a/sha256-hasher/src/lib.rs
+++ b/sha256-hasher/src/lib.rs
@@ -11,14 +11,17 @@ pub struct Hasher {
 
 #[cfg(all(feature = "sha2", not(target_os = "solana")))]
 impl Hasher {
+    #[inline(always)]
     pub fn hash(&mut self, val: &[u8]) {
         self.hasher.update(val);
     }
+    #[inline(always)]
     pub fn hashv(&mut self, vals: &[&[u8]]) {
         for val in vals {
             self.hash(val);
         }
     }
+    #[inline(always)]
     pub fn result(self) -> Hash {
         let bytes: [u8; solana_hash::HASH_BYTES] = self.hasher.finalize().into();
         bytes.into()

Is correct and works. I apologize for the confusion.

[t-nelson]

[roryharr]

anza-xyz/solana-sdk#426

[steviez]

I'm attempting to recreate your results. I did the following to get environment setup:

$ ls
agave
$ git clone https://github.com/anza-xyz/solana-sdk.git
$ cd solana-sdk
$ git checkout sha256-hasher@v3.0.0

and then applied the below patch:

sdk patch

diff --git a/sha256-hasher/Cargo.toml b/sha256-hasher/Cargo.toml
index 3122aa63..05ad4045 100644
--- a/sha256-hasher/Cargo.toml
+++ b/sha256-hasher/Cargo.toml
@@ -16,7 +16,7 @@ targets = ["x86_64-unknown-linux-gnu"]
 sha2 = ["dep:sha2"]
 
 [dependencies]
-solana-hash = { workspace = true }
+solana-hash = "3.0.0"
 
 [target.'cfg(not(target_os = "solana"))'.dependencies]
 sha2 = { workspace = true, optional = true }
diff --git a/sha256-hasher/src/lib.rs b/sha256-hasher/src/lib.rs
index e8785730..39261a8f 100644
--- a/sha256-hasher/src/lib.rs
+++ b/sha256-hasher/src/lib.rs
@@ -11,14 +11,17 @@ pub struct Hasher {
 
 #[cfg(all(feature = "sha2", not(target_os = "solana")))]
 impl Hasher {
+    #[inline(always)]
     pub fn hash(&mut self, val: &[u8]) {
         self.hasher.update(val);
     }
+    #[inline(always)]
     pub fn hashv(&mut self, vals: &[&[u8]]) {
         for val in vals {
             self.hash(val);
         }
     }
+    #[inline(always)]
     pub fn result(self) -> Hash {
         let bytes: [u8; solana_hash::HASH_BYTES] = self.hasher.finalize().into();
         bytes.into()

Back in Agave, I then made the following change to use that version instead of crates.io:

agave patch

diff --git a/Cargo.toml b/Cargo.toml
index d4c2db8e3c..e682d89540 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -520,7 +520,7 @@ solana-send-transaction-service = { path = "send-transaction-service", version =
 solana-serde = "3.0.0"
 solana-serde-varint = "3.0.0"
 solana-serialize-utils = "3.1.0"
-solana-sha256-hasher = "3.0.0"
+solana-sha256-hasher = { path = "../solana-sdk/sha256-hasher", features = ["sha2"] }
 solana-short-vec = "3.0.0"
 solana-shred-version = "3.0.0"
 solana-signature = { version = "3.1.0", default-features = false 

I then ran the following commands from within agave/poh with various versions (and tweaks to keep compiler happy along the way):

// 1.86 baseline
$ cargo +nightly-2025-02-16 bench bench_poh_hash
test bench_poh_hash                   ... bench:   1,393,308.20 ns/iter (+/- 2,866.01)

// 1.86 with local SDK (inlining)
$ cargo +nightly-2025-02-16 bench bench_poh_hash
test bench_poh_hash                   ... bench:   1,393,441.50 ns/iter (+/- 3,174.11)

// 1.87 baseline
$ cargo +nightly-2025-03-29 bench bench_poh_hash 
test bench_poh_hash                   ... bench:   2,188,912.27 ns/iter (+/- 1,756.04)

// 1.87 with local SDK (inlining)
$ cargo +nightly-2025-03-29 bench bench_poh_hash
test bench_poh_hash                   ... bench:   2,085,895.42 ns/iter (+/- 1,387.57)

// 1.88 baseline
$ cargo +nightly-2025-05-09 bench bench_poh_hash
test bench_poh_hash                   ... bench:   2,184,033.60 ns/iter (+/- 986.79)

// 1.88 with local SDK (inlining)
$ cargo +nightly-2025-05-09 bench bench_poh_hash
test bench_poh_hash                   ... bench:   2,077,789.30 ns/iter (+/- 2,250.02)

// 1.89 baseline
cargo +nightly-2025-06-23 bench bench_poh_hash
test bench_poh_hash                   ... bench:   1,470,972.60 ns/iter (+/- 1,639.81)

// 1.89 with local SDK (inlining)
cargo +nightly-2025-06-23 bench bench_poh_hash
test bench_poh_hash                   ... bench:   1,397,362.70 ns/iter (+/- 1,237.45)

So, 1.87 with the SDK change is better but still shows a degradation. @roryharr - Can you try running with 1.87 explicitly to see if you see similar ? As discussed in DM's, you were using latest nightly which is 1.93; we're a bit away from being able to jump to that.

Edit: I have updated to run some more versions and also see 1.89 as the first version which gets us back down to right around where 1.86 is performance wise

[roryharr]

Yes, I can reproduce your results. 1.87 sucks.

test bench_poh_hash                   ... bench:   2,090,148.08 ns/iter (+/- 8,475.99)

[roryharr]

I checked using a few different versions:

// 1.88 ("nightly-2025-05-09")
test bench_poh_hash                   ... bench:   2,073,286.50 ns/iter (+/- 3,783.54)

// 1.89 ("nightly-2025-06-20")
test bench_poh_hash                   ... bench:   1,396,401.20 ns/iter (+/- 1,751.81)

// 1.90 ("nightly-2025-08-01")
test bench_poh_hash                   ... bench:   1,399,967.60 ns/iter (+/- 1,371.46)

// 1.91 ("nightly-2025-09-12") 
test bench_poh_hash                   ... bench:   1,348,958.60 ns/iter (+/- 1,938.01)

So it looks like the regression was fixed in 1.89 nightly and the improvement was introduced in 1.9*, likely by the LLVM compiler upgrade as it was upgraded in 1.91. It is unknown what fixed the regression in the 1.89 nightly.