Cache list account results

[koresar]

Hello

We've noticed that these calls are taking too much time. Need to cache.

I'm looking for a quick turnaround for this PR.

[Uzlopak]

fromDistinctToAccounts seems a little bit computational heavy.
Would like to see a benchmark for this function.

can we reduce the pressure, by reusing prev bytaking it out of the for loop and doing a prev.length = 0?

Also the join call is probably also expensive.

It makes more sense to it like this:

Suggested change

	function fromDistinctToAccounts(distinctResult: string[]) {
	const accountsSet: Set<string> = new Set();
	for (const result of distinctResult) {
	const prev: string[] = [];
	const paths: string[] = result.split(":");
	for (const acct of paths) {
	prev.push(acct);
	accountsSet.add(prev.join(":"));
	}
	}
	return Array.from(accountsSet);
	}
	function fromDistinctToAccounts(distinctResult: string[]) {
	const accountsSet: Set<string> = new Set();
	let i = 0;
	for (const result of distinctResult) {
	const paths: string[] = result.split(":");
	for (i = 0; i < paths.length; ++i) {
	switch (i) {
	case 0:
	accountsSet.add(paths[0]);
	break;
	case 1:
	accountsSet.add(`${paths[0]}:${paths[1]});
	break;
	case 2:
	accountsSet.add(`${paths[0]}:${paths[1]:${paths[2]});
	break;
	case 3:
	accountsSet.add(`${paths[0]}:${paths[1]:${paths[2]:${paths[3]});
	break;
	default:
	// is it till i or does it need to be i -1 or i+1?
	accountsSet.add(paths.slice(0, i).join(':'));
	}
	}
	return Array.from(accountsSet);
	}

untested. But it should reduce some pressure because we avoid alot of intermediary arrays.

[koresar]

Suggestion listened. However, I'd implemented even simpler and faster solution:

function fromDistinctToAccounts(distinctResult: string[]) {
  const accountsSet: Set<string> = new Set();
  for (const fullAccountName of distinctResult) {
    const paths = fullAccountName.split(":");
    let path = paths[0];
    accountsSet.add(path);
    for (let i = 1; i < paths.length; ++i) {
      path += ":" + paths[i];
      accountsSet.add(path);
    }
  }
  return Array.from(accountsSet);
}

[koresar]

Also, this function will be called very occasionally. There is no point optimising it.

[Uzlopak]

maybe better name. isSnapshotObsolete ?

[koresar]

great suggestion!

[Uzlopak]

hmm. why is the return here? iIs it here because then the .catch() can handle it?

[koresar]

yes, correct.
I'll remove the return keyword and use the implicit return of arrow functions.

[Uzlopak]

@koresar

I have reviewed it in regards of potential performance improvements. But it is quiete complicated piece of code. I can not say, that I fully understood it in terms of reliability or avoiding race conditions, which could result in showing wrong account balances or so.

Do you have some lines which you consider being critically checked.

[koresar]

This is not about balances, but about returning the list of all accounts this book has. So, balances won't be affected or touched.

The logic for listAccounts() was copied from the balance() function. The appropriate unit tests were also copied from the balance() method unit tests.

Anyhow, this is a very safe PR for existing users. But addresses a major pain point we have atm.

[koresar]

Published as v6.3