Skip to content

Conversation

@raeidish
Copy link
Contributor

@raeidish raeidish commented Oct 15, 2023

Closes #324

It works but it's not always faster, the sweet spot where the multi is faster seems to be using 512 shards. I tried using batches and getting all keys at once.

@raeidish
Copy link
Contributor Author

if you find anyway to improve it please let me know! i tried a few things but can't seem to get it faster.
@cristaloleg, @janisz

@raeidish
Copy link
Contributor Author

raeidish commented Oct 15, 2023

I forgot to add the locked shard to the map fixing it made a big difference

Copy link
Collaborator

@cristaloleg cristaloleg left a comment

Choose a reason for hiding this comment

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

Please reformat with gofmt

@codecov-commenter
Copy link

codecov-commenter commented Oct 16, 2023

Codecov Report

Merging #379 (e894b71) into main (58fe2d2) will increase coverage by 0.85%.
The diff coverage is 100.00%.

❗ Current head e894b71 differs from pull request most recent head c366663. Consider uploading reports for the commit c366663 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #379      +/-   ##
==========================================
+ Coverage   88.66%   89.52%   +0.85%     
==========================================
  Files          15       15              
  Lines         794      821      +27     
==========================================
+ Hits          704      735      +31     
+ Misses         76       73       -3     
+ Partials       14       13       -1     
Files Coverage Δ
bigcache.go 94.07% <100.00%> (+1.11%) ⬆️
shard.go 92.10% <100.00%> (+1.61%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58fe2d2...c366663. Read the comment docs.

raeidish and others added 3 commits October 17, 2023 10:42
@raeidish raeidish requested a review from janisz October 17, 2023 13:42
@raeidish
Copy link
Contributor Author

Got a bit of a locking issue, when stats is enabled the shard tries to wlock but as it is now it’s already rlocked. This causes a deadlock issue, I’m thinking about moving the hit call outside getWithoutLock.

@raeidish
Copy link
Contributor Author

seems like my test was failing because it was too big for the github action 🙂 looks ok now though.

@raeidish
Copy link
Contributor Author

This is ready for another review 🙂 @janisz

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

4 participants