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

FeatureRequest: provide PinnedSlice or something to avoid memmove #230

Open
choleraehyq opened this issue Jun 22, 2020 · 7 comments
Open

Comments

@choleraehyq
Copy link

For now, every time we get an entry from bigcache when cache hit, bigcache will allocate a new
byte slice and copy cache content into it internally. I propose to provide a way to let users to manage the lifetime of the entry they got, to avoid internal memory copy.

type PinnedSlice struct {...}
func (this PinnedSlice) Copy() PinnedSlice {...}
func (this PinnedSlice) Release() {...}
func (c *BigCache) GetPinnedSlice(key string) (PinnedSlice, error)

Internally, we can add a reference count in the header of wrapped entry, just like timestamps. Once we want to evict an entry, we need to check whether its reference count is zero, if not, we should jump across it and check next entry.

@cristaloleg
Copy link
Collaborator

How this is related to #229 ?

@choleraehyq
Copy link
Author

@cristaloleg It's related but not totally equal. If this proposal is implemented, we don't need #229 anymore.

@siennathesane
Copy link
Collaborator

How would this work with evictions?

@choleraehyq
Copy link
Author

@mxplusb Eviction will be a little bit complex. As I mentioned in the description, when we want to evict an entry, we need to check the reference count first. So we need to find a continuous memory long enough to fit in the new entry and all reference counts of old entries in this continuous memory need to be zero. Compare to the size of the whole cache, we can assume most of the cached entries will have zero reference count, so the eviction procedure should not be too expansive.

@siennathesane
Copy link
Collaborator

I can review a PR if you'd like. I don't inherently see a problem with the API, but I would defer to @cristaloleg for whether or they'd want to move forward with it.

@rupor-github
Copy link

If implemented evictions rather than being a single pop operation could potentially take n hops - to find next node to evict. Today queue head always points to the latest node - this I guess would not be true any longer?

@siennathesane
Copy link
Collaborator

I think this might be a good candidate for v3, but I would want to see a reference implementation before saying yea or nay. I think the value of a PinnedSlice or something equivalent could be interesting, but I think it should also be clearly explained on when to use it vs when to not.

// ref: #238

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

No branches or pull requests

4 participants