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

Remove LocalCache in FileStore #42626

Merged
merged 1 commit into from
Jul 28, 2021

Conversation

casperisfine
Copy link
Contributor

Ref: #42611

Context

LocalCache used to keep the deserialized value in memory, however this opened the door for the cached value to be mutated (see #36656 and #37587). So it used to both save the backend rountrip, and the deserialization cost.

But now that we almost always need to deserialize local cache entries, LocalCache now at best only save the backend rountrip. And in the case of the FileStore it's pretty much nothing.

Benchmark

I used a slightly modified version of @hoangtuyb96's benchmark: https://gist.github.com/casperisfine/80620e26aaf786a9d3016d5823ade15b

$ DISABLE_LOCAL_CACHE=1 SIZE=10 ruby bench-as-cache.rb 
fetch in rails 7.0.0.alpha
                        258.287  (± 1.5%) i/s -      1.300k in   5.034323s
write in rails 7.0.0.alpha
                          1.776k (± 3.3%) i/s -      8.960k in   5.050029s
$ SIZE=10 ruby bench-as-cache.rb 
fetch in rails 7.0.0.alpha
                        233.276  (± 6.9%) i/s -      1.175k in   5.065071s
write in rails 7.0.0.alpha
                          1.476k (±10.3%) i/s -      7.238k in   5.030478s

$ DISABLE_LOCAL_CACHE=1 SIZE=1000 ruby bench-as-cache.rb 
fetch in rails 7.0.0.alpha
                        228.549  (± 6.6%) i/s -      1.144k in   5.026979s
write in rails 7.0.0.alpha
                        129.034  (± 2.3%) i/s -    648.000  in   5.024184s

$ SIZE=1000 ruby bench-as-cache.rb
fetch in rails 7.0.0.alpha
                        234.919  (± 7.2%) i/s -      1.170k in   5.011433s
write in rails 7.0.0.alpha
                         67.769  (± 1.5%) i/s -    342.000  in   5.048530s

So LocalCache is pretty much useless for FileStore.

@casperisfine casperisfine force-pushed the as-file-store-remove-local-cache branch from c0a5d05 to a2148b9 Compare June 28, 2021 13:22
@rails-bot rails-bot bot added the railties label Jun 28, 2021
@byroot byroot merged commit f7cedc6 into rails:main Jul 28, 2021
ghiculescu added a commit to ghiculescu/rails that referenced this pull request Mar 2, 2022
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.

None yet

2 participants