Skip to content

Conversation

@Szymon20000
Copy link
Contributor

@Szymon20000 Szymon20000 commented Sep 23, 2023

We found out that getAllKeys can be extremely slow as it's used very frequently and it just copies keys from cache to just created array. Because on heavy account number of keys can easily reach 50k that takes quite some time.
In most of the cases we only ask for one keys so to speed it up we just check if it's in the set of keys if it is then we return array with just this item.

Details

Related Issues

GH_LINK

Automated Tests

Linked PRs

@Szymon20000 Szymon20000 marked this pull request as ready for review September 27, 2023 11:38
@Szymon20000 Szymon20000 requested a review from a team as a code owner September 27, 2023 11:38
@melvin-bot melvin-bot bot requested review from youssef-lr and removed request for a team September 27, 2023 11:39
Copy link
Contributor

@mountiny mountiny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@mountiny mountiny merged commit 74245ab into Expensify:main Sep 28, 2023
@ospfranco ospfranco mentioned this pull request Sep 28, 2023
59 tasks
&& !(mapping.key.endsWith('_'))
&& cache.storageKeys.has(mapping.key)
) {
return [mapping.key];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB, but now that we do this - some of the comments below are a little misleading. Since this is not a key that we found (we just assumed it exists).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants