Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance of memoized validator functions. #129

Merged
merged 5 commits into from
Sep 21, 2024

Conversation

leblowl
Copy link
Contributor

@leblowl leblowl commented Sep 13, 2024

Replaces #122 to enable edits by maintainers.

The cache key was based on a hash of the entire graph. This hash was being
generated each time a link was validated. On my system, validating each link
took around 100ms with a graph of around 2000 links. With this change it appears
that validating each link takes less than 1ms.
@HerbCaudill
Copy link
Member

Thanks @leblowl ! I've removed memoization for the individual validators and memoized the validate function instead. It still uses a hash of the graph as the memoization key, but the performance is equivalent to your original change.

benchmark on #122 :

 ✓ packages/auth/src/test/auth.benchmark.ts (1) 3387ms
   ✓ auth (1) 3386ms
     name                      hz     min     max    mean     p75     p99    p995    p999     rme  samples
   · a new member joining  5.0861  178.43  221.59  196.61  209.11  221.59  221.59  221.59  ±5.26%       10

benchmark on this PR:

 ✓ packages/auth/src/test/auth.benchmark.ts (1) 3429ms
   ✓ auth (1) 3428ms
     name                      hz     min     max    mean     p75     p99    p995    p999     rme  samples
   · a new member joining  5.0062  180.69  216.99  199.75  212.02  216.99  216.99  216.99  ±4.39%       10

Let me know what you think, if this looks good to you I'll go ahead and merge it.

@HerbCaudill HerbCaudill merged commit 23f11af into local-first-web:main Sep 21, 2024
1 check passed
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.

2 participants