pinocchio: Add pubkey_eq helper

[febo]

Problem

In some cases, comparing pubkeys with the equality operator (==) results in "bloated" code, which eventually leads to an increased CU consumption.

Solution

This PR adds a pubkey_eq helper, which compares pubkeys as a sequence of u64 values.

[sonicfromnewyoke]

what do you think about impl PartialEq for Pubkey in the sdk?
to have API like follows:

mint_a.key().eq(mint_b.key())

[febo]

what do you think about impl PartialEq for Pubkey in the sdk? to have API like follows:

mint_a.key().eq(mint_b.key())

Yeah, definitely – we should have it on the Address crate.

[rustopian]

Looks good to me. One question on a use of unlikely.

[rustopian]

Looks great.

[ilya-bobyr]

If this is just a more efficient implementation of ==, it seems better to redefine PartialEq:

// Comparing `Pubkey`s 8 bytes at a time is faster, than comparing byte at a
// time.  One would expect LLVM to do this optimization for us, but it does not
// at the moment (as of platform tools v1.43).  Comparing keys is a pretty
// common operation, so we want to make it faster if we can.
impl PartialEq for Pubkey {
    fn eq(&self, other: &Self) -> bool {
        let p1 = self.as_ptr() as *const u64;
        let p2 = other.as_ptr() as *const u64;

        // SAFETY: `p1` and `p2` both point to `[u8; 32]` bytes of data, which
        // is the same size as `[u64; 4]`.
        unsafe {
            read_unaligned(p1) == read_unaligned(p2)
                && read_unaligned(p1.add(1)) == read_unaligned(p2.add(1))
                && read_unaligned(p1.add(2)) == read_unaligned(p2.add(2))
                && read_unaligned(p1.add(3)) == read_unaligned(p2.add(3))
        }
    }
}

Bikeshedding

Maybe it is possible to get the same effect if you just help the compiler a bit,
by casting [u8; 32] into [u64; 4]?

// Comparing `Pubkey`s 8 bytes at a time is faster than comparing byte at a
// time.  One would expect LLVM to do this optimization for us, but it does not
// at the moment (as of platform tools v1.43).  Comparing keys is a pretty
// common operation, so we want to make it faster if we can.
impl PartialEq for Pubkey {
    fn eq(&self, other: &Self) -> bool {
        let p1 = self.as_ptr() as *const [u64; 4];
        let p2 = other.as_ptr() as *const [u64; 4];

        // SAFETY: `p1` and `p2` both point to `[u8; 32]` bytes of data, which
        // is the same size as `[u64; 4]`.
        unsafe { read_unaligned(p1) == read_unaligned(p2) }
    }
}

I would also add static_assertions, to be able to encode the SAFETEY comment
in code:

use static_assertions::assert_eq_size;

// Comparing `Pubkey`s 8 bytes at a time is faster than comparing byte at a
// time.  One would expect LLVM to do this optimization for us, but it does not
// at the moment (as of platform tools v1.43).  Comparing keys is a pretty
// common operation, so we want to make it faster if we can.
impl PartialEq for Pubkey {
    fn eq(&self, other: &Self) -> bool {
        let p1 = self.as_ptr() as *const [u64; 4];
        let p2 = other.as_ptr() as *const [u64; 4];

        assert_eq_size!(*self, [u64; 4]);
        assert_eq_size!(*other, [u64; 4]);
        // SAFETY: `self` and `[u64; 4]` are of the same size.
        unsafe { read_unaligned(p1) == read_unaligned(p2) }
    }
}

(It is a compile time assertion with no runtime code.)

[rustopian]

I wanted this, too – at first. It would be great to get the savings everywhere devs use ==.

But since our pinocchio::pubkey::Pubkey is just an alias for [u8;32], we can't impl PartialEq on it. Creating a new type is maybe not worth it and probably a separate discussion.

Also, sounds like Lucas is looking into this, so maybe this discussion will be irrelevant soon.

[febo]

If this is just a more efficient implementation of ==, it seems better to redefine PartialEq:

We have done this for the solana-address here, since Address is a type.

Bikeshedding

Maybe it is possible to get the same effect if you just help the compiler a bit, by casting [u8; 32] into [u64; 4]?

Unfortunately no, it is even worse. 😅 I think the compiler tries to do something different when it sees an array, which in the end becomes "bloated" and uses more CU. You can use this simple bench to see the effect.

As @rustopian mentioned, Lucas will have a look at this to see if we can improve the code generation so the compiler does the "right" thing.

[ilya-bobyr]

I wanted this, too – at first. It would be great to get the savings everywhere devs use ==.

But since our pinocchio::pubkey::Pubkey is just an alias for [u8;32], we can't impl PartialEq on it. Creating a new type is maybe not worth it and probably a separate discussion.

This is out of scope for this PR obviously, but, considering Pubkey is such a fundamental abstraction, I think it should be a type.

[febo]

It will be very soon: #118 😊