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.

Sign up for GitHub

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

Jump to bottom

[Fixes #11512] improves cache size calculation in MemoryStore #11527

Closed
wants to merge 1 commit into from
Closed

[Fixes #11512] improves cache size calculation in MemoryStore #11527

wants to merge 1 commit into from

Conversation

ssimeonov
Copy link
Contributor

@drogus
Copy link
Member

drogus commented Jul 21, 2013

@ssimeonov thanks for a PR. it looks good apart from 2 things:

  • please add a changelog entry
  • please add a detailed description in the commit message instead of linking to it - people should not be forced to open the browser when looking at the git history (more info on commiting and messages is here: guides.rubyonrails.org/contributing_to_ruby_on_rails.html#commit-your-changes)

@ssimeonov
[Fixes rails#11512] improves cache size calculation in ActiveSupport:…
fc7f7d0 
…:Cache::MemoryStore

Improves `ActiveSupport::Cache::MemoryStore` cache size calculation.
The size of a key/entry pair is calculated via `#cached_size`:

    def cached_size(key, entry)
      key.to_s.bytesize + entry.size + PER_ENTRY_OVERHEAD
    end

The value of `PER_ENTRY_OVERHEAD` is 240 bytes based on an [empirical
estimation](https://gist.github.com/ssimeonov/6047200) for 64-bit MRI on
1.9.3 and 2.0.

Fixes GH#11512 rails#11512
@ssimeonov
Copy link
Contributor Author

@drogus Done + will do a separate pull request with a single, clean commit.

@ssimeonov ssimeonov closed this Jul 22, 2013
@ssimeonov ssimeonov mentioned this pull request Jul 22, 2013
Merged
@steveklabnik
Copy link
Member

You don't need to make new PRs, just rebase and force push.

@ssimeonov
Copy link
Contributor Author

@steveklabnik yes, that's what I did and you can see it in the commit log of the forked repo. GH had a hiccup of sorts and showed me two commits as opposed to a single one in this PR, which is why I closed it and created a new one. I presume it was some type of ephemeral fragment caching.

@steveklabnik
Copy link
Member

Cool. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ssimeonov @drogus @steveklabnik