solana-labs / solana

Web-Scale Blockchain for fast, secure, scalable, decentralized apps and marketplaces.
https://solanalabs.com
Apache License 2.0
13.05k stars 4.21k forks source link

web3.js: Memoize `toBase58` #25644

Closed steveluscher closed 2 years ago

steveluscher commented 2 years ago

Problem

There's a class of application that needs to display and/or compare keys, a lot. Every time they call toBase58 on PublicKey, we burn another bit of CPU and use another chunk of memory.

Proposed Solution

Memoize toBase58. Consuming memory is probably better than burning CPU, especially for render-heavy applications like Solana Explorer.

ftelnov commented 2 years ago

It can be done via proxying and through dozens other methods. There is no reason to add caching on the library level.

steveluscher commented 2 years ago

I'm curious! Can you explain more?

ftelnov commented 2 years ago

Make a substitution for specific toBase58 method. It can be done via Proxy. Guides can be found on the net ofc. If I can remember things properly, there is also possibility to insert additional fields in Proxy objects.

jordaaash commented 2 years ago

I think this optimization should be done in web3.js. Many users of all libraries built on top of web3.js are affected by this.

For example: https://github.com/metaplex-foundation/metaplex/search?q=toBase58

Here, an implementation detail and a cheap fix. Anywhere else it's much more involved to refactor.

steveluscher commented 2 years ago

I'm closing this issue, not because it's done, but because we probably shouldn't introduce this to the core library.

Folks' use cases vary too widely for us to make one tradeoff that suits everyone, and nobody wants more bloat in web3.js or to have to understand yet-another configuration option for something as small as toBase58().

The good news is that someone can easily make a PublicKey memoizing stringifier in userspace, and configure in the tradeoff that's best for their app:

import MyFavoriteLRUCache from '...';

function getCacheKey(publicKey: PublicKey) {
  // My favorite way to produce a cache key from a `PublicKey` instance.
}

let cache;
function getCache() {
  if (cache === undefined) {
    cache = new MyFavoriteLRUCache({
      size: 5000, /* An appropriate size for your app */
    })
  }
  return cache;
}

export default getBase58StringFromPublicKey(publicKey: PublicKey): string {
  const cache = getCache();
  const cacheKey = getCacheKey(publicKey);
  const cachedBase58String = cache.get(cacheKey);
  if (cachedBase58String) {
    return cachedBase58String;
  }
  const freshBase58String = publicKey.toBase58();
  cache.set(cacheKey, freshBase58String);
  return freshBase58String; 
}

…later…

import getBase58StringFromPublicKey from 'getBase58StringFromPublicKey';

export default function ProgramAddress(props) {
  return (
    <div>
      {getBase58StringFromPublicKey(props.publicKey)}
    </div>
  );
}

I'm going to try to make all of this a non-problem in the rewrite of web3.js. See principle 4 about opaque types; all pubkeys should just be interned JavaScript strings that share memory naturally, and are validated by TypeScript opaque types.