Skip to content

Commit

Permalink
cache improvements
Browse files Browse the repository at this point in the history
- update the descriptors/inodes of a PID when it's found in cache.
- when a descriptor/inode is found in cache, push it to the top
  of the descriptors list. The next time it's found in cache it'll be in
  the 1st position of the list, saving CPU time.
- added test cases and benchmark helpers to help analyzing performance.
  • Loading branch information
gustavo-iniguez-goya committed Mar 19, 2021
1 parent b292838 commit 1a61a2d
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 13 deletions.
31 changes: 23 additions & 8 deletions daemon/procmon/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ var (
func addProcEntry(fdPath string, fdList []string, pid int) {
for n := range pidsCache {
if pidsCache[n].Pid == pid {
pidsCache[n].Descriptors = fdList
pidsCache[n].Time = time.Now()
return
}
Expand Down Expand Up @@ -105,36 +106,50 @@ func getPidByInodeFromCache(inodeKey string) int {
return -1
}

func getPidDescriptorsFromCache(pid int, fdPath string, expect string, descriptors []string) int {
for fdIdx := 0; fdIdx < len(descriptors); fdIdx++ {
descLink := fmt.Sprint(fdPath, descriptors[fdIdx])
func getPidDescriptorsFromCache(fdPath string, expect string, descriptors *[]string) (int, *[]string) {
for fdIdx := 0; fdIdx < len(*descriptors); fdIdx++ {
descLink := fmt.Sprint(fdPath, (*descriptors)[fdIdx])
if link, err := os.Readlink(descLink); err == nil && link == expect {
return fdIdx
if fdIdx > 0 {
// reordering helps to reduce look up times by a factor of 10.
fd := (*descriptors)[fdIdx]
*descriptors = append((*descriptors)[:fdIdx], (*descriptors)[fdIdx+1:]...)
*descriptors = append([]string{fd}, *descriptors...)
}
return fdIdx, descriptors
}
}

return -1
return -1, descriptors
}

func getPidFromCache(inode int, inodeKey string, expect string) (int, int) {
// loop over the processes that have generated connections
for n := 0; n < len(pidsCache); n++ {
procEntry := pidsCache[n]

if idxDesc := getPidDescriptorsFromCache(procEntry.Pid, procEntry.FdPath, expect, procEntry.Descriptors); idxDesc != -1 {
if idxDesc, newFdList := getPidDescriptorsFromCache(procEntry.FdPath, expect, &procEntry.Descriptors); idxDesc != -1 {
pidsCache[n].Time = time.Now()
pidsCache[n].Descriptors = *newFdList
return procEntry.Pid, n
}
}
// inode not found in cache, we need to refresh the list of descriptors
// to see if any of the known PIDs has opened a new socket, and update
// the new list of file descriptors for that PID.

descriptors := lookupPidDescriptors(procEntry.FdPath)
for n := 0; n < len(pidsCache); n++ {
procEntry := pidsCache[n]
descriptors := lookupPidDescriptors(procEntry.FdPath, procEntry.Pid)
if descriptors == nil {
deleteProcEntry(procEntry.Pid)
continue
}

pidsCache[n].Descriptors = descriptors
if idxDesc := getPidDescriptorsFromCache(procEntry.Pid, procEntry.FdPath, expect, descriptors); idxDesc != -1 {
if idxDesc, newFdList := getPidDescriptorsFromCache(procEntry.FdPath, expect, &descriptors); idxDesc != -1 {
pidsCache[n].Time = time.Now()
pidsCache[n].Descriptors = *newFdList
return procEntry.Pid, n
}
}
Expand Down
71 changes: 71 additions & 0 deletions daemon/procmon/cache_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package procmon

import (
"fmt"
"testing"
)

func TestCacheProcs(t *testing.T) {
fdList := []string{"0", "1", "2"}
addProcEntry(fmt.Sprint("/proc/", myPid, "/fd/"), fdList, myPid)
t.Log("Pids in cache: ", len(pidsCache))

t.Run("Test addProcEntry", func(t *testing.T) {
if len(pidsCache) != 1 {
t.Error("pidsCache should be 1")
}
})

oldPid := pidsCache[0]
addProcEntry(fmt.Sprint("/proc/", myPid, "/fd/"), fdList, myPid)
t.Run("Test addProcEntry update", func(t *testing.T) {
if len(pidsCache) != 1 {
t.Error("pidsCache should be still 1!", pidsCache)
}
if oldPid.Time.Equal(pidsCache[0].Time) == false {
t.Error("pidsCache, time not updated: ", oldPid.Time, pidsCache[0].Time)
}
})

addProcEntry("/proc/2/fd/", fdList, 2)
deleteProcEntry(2)
t.Run("Test deleteProcEntry", func(t *testing.T) {
if len(pidsCache) != 1 {
t.Error("pidsCache should be 1:", len(pidsCache))
}
})

pid, _ := getPidFromCache(0, "", "/dev/null")
t.Run("Test getPidFromCache", func(t *testing.T) {
if pid != myPid {
t.Error("pid not found in cache", len(pidsCache))
}
})

for pid := 3; pid < 27; pid++ {
addProcEntry(fmt.Sprint("/proc/", pid, "/fd/"), fdList, pid)
}
if len(pidsCache) != 25 {
t.Error("pidsCache should be 0:", len(pidsCache))
}
cleanUpCaches()
t.Run("Test cleanUpCaches", func(t *testing.T) {
if len(pidsCache) != 0 {
t.Error("pidsCache should be 0:", len(pidsCache))
}
})
}

// Test getPidDescriptorsFromCache descriptors (inodes) reordering.
// When an inode (descriptor) is found, if it's pushed to the top of the list,
// the next time we look for it will cost -10x.
// Without reordering, the inode 0 will always be found on the 10th position,
// taking an average of 100us instead of 30.
// Benchmark results with reordering: ~5600ns/op, without: ~56000ns/op.
func BenchmarkGetPid(b *testing.B) {
fdList := []string{"10", "9", "8", "7", "6", "5", "4", "3", "2", "1", "0"}
addProcEntry(fmt.Sprint("/proc/", myPid, "/fd/"), fdList, myPid)
for i := 0; i < b.N; i++ {
getPidFromCache(0, "", "/dev/null")
}
}
9 changes: 7 additions & 2 deletions daemon/procmon/find.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func sortPidsByTime(fdList []os.FileInfo) []os.FileInfo {
// If the inode is found, the cache is updated ans sorted.
func inodeFound(pidsPath, expect, inodeKey string, inode, pid int) bool {
fdPath := fmt.Sprint(pidsPath, pid, "/fd/")
fdList := lookupPidDescriptors(fdPath)
fdList := lookupPidDescriptors(fdPath, pid)
if fdList == nil {
return false
}
Expand Down Expand Up @@ -57,11 +57,16 @@ func lookupPidInProc(pidsPath, expect, inodeKey string, inode int) int {
// lookupPidDescriptors returns the list of descriptors inside
// /proc/<pid>/fd/
// TODO: search in /proc/<pid>/task/<tid>/fd/ .
func lookupPidDescriptors(fdPath string) []string {
func lookupPidDescriptors(fdPath string, pid int) []string {
f, err := os.Open(fdPath)
if err != nil {
return nil
}
// This is where most of the time is wasted when looking for PIDs.
// long running processes like firefox/chrome tend to have a lot of descriptor
// references that points to non existent files on disk, but that remains in
// memory (those with " (deleted)").
// This causes to have to iterate over 300 to 700 items, that are not sockets.
fdList, err := f.Readdir(-1)
f.Close()
if err != nil {
Expand Down
17 changes: 14 additions & 3 deletions daemon/procmon/find_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ func TestGetProcPids(t *testing.T) {
}

func TestLookupPidDescriptors(t *testing.T) {
pidsFd := lookupPidDescriptors(fmt.Sprint("/proc/", myPid, "/fd/"))

pidsFd := lookupPidDescriptors(fmt.Sprint("/proc/", myPid, "/fd/"), myPid)
if len(pidsFd) == 0 {
t.Error("getProcPids() should not be 0", pidsFd)
}
Expand All @@ -24,8 +23,20 @@ func TestLookupPidDescriptors(t *testing.T) {
func TestLookupPidInProc(t *testing.T) {
// we expect that the inode 1 points to /dev/null
expect := "/dev/null"
foundPid := lookupPidInProc("/proc/", expect, "", 1)
foundPid := lookupPidInProc("/proc/", expect, "", myPid)
if foundPid == -1 {
t.Error("lookupPidInProc() should not return -1")
}
}

func BenchmarkGetProcs(b *testing.B) {
for i := 0; i < b.N; i++ {
getProcPids("/proc")
}
}

func BenchmarkLookupPidDescriptors(b *testing.B) {
for i := 0; i < b.N; i++ {
lookupPidDescriptors(fmt.Sprint("/proc/", myPid, "/fd/"), myPid)
}
}

0 comments on commit 1a61a2d

Please sign in to comment.