Only run GC if the memory has shrunk or expanded by more than 0.5GB. #4075
Conversation
The GC is being manually called too many times. This change reduces the number of times it's run by only calling it if the allocated memory in the heap has shrunk or expanded by more than half a GB and if it's not being run in the last ten seconds.
![pullrequest[bot]](https://avatars.githubusercontent.com/in/3434?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ A review job has been created and sent to the PullRequest network.
@martinmr you can click here to see the review status or cancel the code review job.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @martinmr)
dgraph/main.go, line 41 at r1 (raw file):
defer ticker.Stop() minDiff := uint64(5e8)
512 << 20
dgraph/main.go, line 55 at r1 (raw file):
} if ms.NumGC >= lastNum {
Remove the equals. Update the if below.
dgraph/main.go, line 62 at r1 (raw file):
// more than 0.5GB since the last time the memory stats were collected. lastNum = ms.NumGC } else {
if ms.NumGC == lastNum
dgraph/main.go
Outdated
| diff = lastMs.HeapAlloc - ms.HeapAlloc | ||
| } | ||
|
|
||
| if ms.NumGC >= lastNum { |
There was a problem hiding this comment.
Same issue with naming here. NumGC doesn't seem intuitive to understand here.
dgraph/main.go
Outdated
| @@ -38,20 +38,35 @@ func main() { | |||
| ticker := time.NewTicker(10 * time.Second) | |||
| defer ticker.Stop() | |||
|
|
|||
| minDiff := uint64(5e8) | |||
| var lastMs runtime.MemStats | |||
| var lastNum uint32 | |||
There was a problem hiding this comment.
It would be useful for this variable name to be a bit more clear such as lastRunTimestamp.
dgraph/main.go
Outdated
| @@ -38,20 +38,35 @@ func main() { | |||
| ticker := time.NewTicker(10 * time.Second) | |||
| defer ticker.Stop() | |||
|
|
|||
| minDiff := uint64(5e8) | |||
There was a problem hiding this comment.
I'm finding that 5e8 is 0.47 GB (2 s.f.). Why this for 0.5 GB?
There was a problem hiding this comment.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @manishrjain and @pullrequest[bot])
dgraph/main.go, line 41 at r1 (raw file):
Previously, manishrjain (Manish R Jain) wrote…
512 << 20
Done.
dgraph/main.go, line 41 at r1 (raw file):
Previously, pullrequest[bot] wrote…
I'm finding that 5e8 is 0.47 GB (2 s.f.). Why this for 0.5 GB?
Done
dgraph/main.go, line 43 at r1 (raw file):
Previously, pullrequest[bot] wrote…
It would be useful for this variable name to be a bit more clear such as
lastRunTimestamp.
Renamed to lastNumGC to match the naming in MemStats
dgraph/main.go, line 55 at r1 (raw file):
Previously, manishrjain (Manish R Jain) wrote…
Remove the equals. Update the if below.
Done.
dgraph/main.go, line 55 at r1 (raw file):
Previously, pullrequest[bot] wrote…
Same issue with naming here.
NumGCdoesn't seem intuitive to understand here.
Cannot rename this variable.
dgraph/main.go, line 62 at r1 (raw file):
Previously, manishrjain (Manish R Jain) wrote…
if ms.NumGC == lastNum
Done.
The GC is being manually called too many times. This change reduces the
number of times it's run by only calling it if the allocated memory in
the heap has shrunk or expanded by more than half a GB and if it's not
being run in the last ten seconds.
This change is