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

Get multiple cached values #379

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

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

bigcache.go Outdated Show resolved Hide resolved
bigcache.go Outdated Show resolved Hide resolved
bigcache_bench_test.go Outdated Show resolved Hide resolved
@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.

shard.go Outdated Show resolved Hide resolved
bigcache.go Outdated Show resolved Hide resolved
shard.go Show resolved Hide resolved
bigcache.go Outdated Show resolved Hide resolved
bigcache.go Outdated Show resolved Hide resolved
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get multiple items in a single function call
4 participants