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

[BUG]: ristretto used about 2 x MaxCost memory?? #320

Open
RyouZhang opened this issue Dec 7, 2022 · 5 comments
Open

[BUG]: ristretto used about 2 x MaxCost memory?? #320

RyouZhang opened this issue Dec 7, 2022 · 5 comments
Labels
kind/bug Something is broken.

Comments

@RyouZhang
Copy link

What version of Ristretto are you using?

0.11

What version of Go are you using?

go 1.19.3

Have you tried reproducing the issue with the latest release?

Yes

What is the hardware spec (RAM, CPU, OS)?

4c/8g Linux 5.4.219-126.411.amzn2.x86_64 #1

What steps will reproduce the bug?

package main

import (
"time"
"fmt"

ro "github.com/dgraph-io/ristretto"
"github.com/google/uuid"

)

func main() {

ch := make(chan bool)

cache, err := ro.NewCache(&ro.Config{
	NumCounters: 1000000,    // number of keys to track frequency of (10M).
	MaxCost:     1024*1024*100, // maximum cost of cache (1GB).
	BufferItems: 64,         // number of keys per Get buffer.
	Metrics: true,
	IgnoreInternalCost:false,
})
if err != nil {
	panic(err)
}

for i:=0; i<10; i++ {
go func() {
	for {
		raw := make([]byte, 1024*16)
		cache.Set(uuid.NewString(), raw, int64(len(raw)))
		<-time.After(1 * time.Millisecond)
	}

}()
}
go func() {
	for {
		fmt.Println("cost:",(cache.Metrics.CostAdded()- cache.Metrics.CostEvicted())/1024.0/1024.0, "MB item:", cache.Metrics.KeysAdded() - cache.Metrics.KeysEvicted())
		<-time.After(1000 * time.Millisecond)
	}
}()
<-ch

}

Expected behavior and actual result.

in my demo, the max-cost is 100MB, but top your pid, you will find it's used 200MB memory

Additional information

No response

@RyouZhang RyouZhang added the kind/bug Something is broken. label Dec 7, 2022
@andys
Copy link

andys commented Jan 6, 2023

You're calling Set() with a cost of 16, but actually storing more than double that, because you are converting UUID from raw bytes to a string ?:

fmt.Println("len = ", len(uuid.NewString()))

len = 36

@RyouZhang
Copy link
Author

RyouZhang commented Jan 7, 2023

You're calling Set() with a cost of 16, but actually storing more than double that, because you are converting UUID from raw bytes to a string ?:

fmt.Println("len = ", len(uuid.NewString()))

len = 36

the UUID is key, the real value is 16K, and I don't is the reason, because memory cost 2x

@andys
Copy link

andys commented Jan 7, 2023

@RyouZhang sorry, my mistake. Have you tried going much bigger, like, at 1GB of byte values, does it use 2GB of memory or just 1.1GB?

@RyouZhang
Copy link
Author

@RyouZhang sorry, my mistake. Have you tried going much bigger, like, at 1GB of byte values, does it use 2GB of memory or just 1.1GB?

yes, in my prod env, we set 3GB but real used 6+GB

@nearsyh
Copy link

nearsyh commented Apr 14, 2023

After some reseach, I think the behaivor of ristretto is "correct". The in-use heap size is indeed ~100MB. I think the reason why the RES memory in top is higher is that the gc'ed memory is not returned to the OS. If we add debug.FreeOSMemory() in the cost printing loop (with a higher frequency, e.g. 1000ms -> 100ms), we can see a lower RES size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something is broken.
Projects
None yet
Development

No branches or pull requests

3 participants