-
Notifications
You must be signed in to change notification settings - Fork 374
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix(jemalloc): Fix the stats of jemalloc #236
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.
Looks good to me.
fmt.Printf("[%d] Current Memory: %05.2f G. Increase? %v\n", | ||
counter, float64(curMem)/float64(1<<30), increase) | ||
var js z.MemStats | ||
z.ReadMemStats(&js) |
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.
Do you need this in memtest?
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.
I was thinking that this can be something which we can use to verify that the stats are as expected.
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.
Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @ahsanbarkati, @jarifibrahim, @manishrjain, and @martinmr)
contrib/memtest/main.go, line 114 at r1 (raw file):
Previously, ahsanbarkati (Ahsan Barkati) wrote…
I was thinking that this can be something which we can use to verify that the stats are as expected.
The memstats can be added on. But, our version of how much memory should be used should still be printed.
z/calloc_jemalloc.go, line 164 at r1 (raw file):
sz := unsafe.Sizeof(&epoch) C.je_mallctl( (C.CString)("epoch"),
Link to documentation about this?
contrib/memtest/main.go, line 114 at r1 (raw file): Previously, manishrjain (Manish R Jain) wrote…
Yep, that is still printed. |
The jemalloc stats are read by using the
mallctl()
call, this call doesn't return the latest stats unless we refresh the ctl cache. We can get the latest stats after calling an epoch mallctl. This PR fixes this issue.Note: Calling an epoch mallctl is expensive. An epoch mallctl takes up all the locks as taken by a malloc call. We should use this judiciously.
This change is