Rpc: possible performance improvement for filters?

[fanatid]

Problem

solana/client/src/rpc_filter.rs

Lines 49 to 53 in ca83167

	#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
	#[serde(rename_all = "camelCase", untagged)]
	pub enum MemcmpEncodedBytes {
	Binary(String),
	}

MemcmpEncodedBytes enum looks not healthy, why Binary variant has String as inner? And more, this String have bytes encoded in base58 😧

I'm not sure about the performance impact but decode base58 string each time for filter account on requests like get_program_accounts looks not good.

Proposed Solution

1. Rename Binary to Base58 and add Binary(Vec<u8>).
2. Left Binary(String) and add new variant with Vec<u8> (name?).

[mvines]

I agree it would be nice to have a new Base64(String) variant of this enum to avoid the base58 entirely. We can't change the existing Binary() name as that'd would break backwards compatibility.

But it's worse! The Binary(String) shouldn't be terrible if the base58 decode only occurs once per RPC call, but this appears to not be the case:

bytes_match() is called for each program account:

solana/rpc/src/rpc.rs

Line 1799 in 122206d

RpcFilterType::Memcmp(compare) => compare.bytes_match(account.data()),

and it internally performs a base58 decode into a local variable

solana/client/src/rpc_filter.rs

Line 69 in 122206d

let bytes = bs58::decode(bytes).into_vec();

...every time. ☠️

It should be a big win to memoize that base58 decode so it only occurs once per filter per RPC request (and even if we added a Base64(String), we'd want to memoize the base64 decode as well)

[fanatid]

If we add BinaryUnderOtherName(Vec<u8>) we would able to convert Base64(String) / Base58(String) / etc to decoded variant before applying filter. Just not sure under which name to add Vec<u8> 😆 .

[mvines]

A Vec<u8> over JSON RPC would be encoded as an array of numbers, which isn't super efficient bandwidth-wise. That's why we favour encodings that end up as a string over the wire generally

[fanatid]

Thanks for clearing this up. I'll try to solve this issue.

[github-actions]

This issue has been automatically locked since there has not been any activity in past 7 days after it was closed. Please open a new issue for related bugs.