-
Notifications
You must be signed in to change notification settings - Fork 3
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
Create basic contentcache test #16
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please combine this PR with #15, we would add a feature along with its test in one PR.
) | ||
|
||
func TestGetContent(t *testing.T) { | ||
cc, err := NewContentCache(128, true) //for some reason Set fails if size <= 60 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? Can we investigate the reason?
size := contentCacheSize | ||
cache, err := ristretto.NewCache(&ristretto.Config{ | ||
//NumCounters is 10 times estimated max number of items in cache, as suggested in https://pkg.go.dev/github.com/dgraph-io/[email protected]#Config | ||
NumCounters: 1e7, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is calculated from number of items, not size of items. I don't think there would be 1000000 files in the cache.
|
||
ce := ContentCacheCell{ | ||
id: id, | ||
Data: bytes.NewBuffer(bytes.Trim(b, string([]byte{0x0}))), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why need trim 0 bytes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because b
is initialized with max cache size, there are 0 bytes after reading
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be problematic:
- Every time we load a file, no matter how large/small it is, we will allocate the max cached file size (~10MB), which makes memory cosumption unnecessarily high.
- The file can ends with 0 bytes, and removing them would be inappropiate.
No description provided.