Skip to content

Commit

Permalink
chore!: rollback proctree to simple LRU (#4299)
Browse files Browse the repository at this point in the history
The expirable LRU cache is not intended to be used in the ProcessTree
type since the TTL would impact its correctness on some scenarios.

Despite TTL can be disabled by setting it to 0, it is not a good idea
due to the potential performance impact of its implementation. See the
comparison between the simple LRU and the expirable LRU below:

| Benchmark                              | Exp LRU |  Simple |    %   |
|                                        |  TTL 0  |   LRU   |  Impr. |
|                                        | (ns/op) | (ns/op) |    *   |
|----------------------------------------|---------|---------|--------|
| GetProcessByHash-Concurrency1          | 4.264   | 6.301   | -47.8% |
| GetProcessByHash-Concurrency2          | 14.91   | 19.85   | -33.2% |
| GetProcessByHash-Concurrency4          | 74.03   | 63.22   | 14.6%  |
| GetProcessByHash-Concurrency8          | 247.2   | 174.7   | 29.3%  |
| GetOrCreateProcessByHash-Concurrency1  | 37.02   | 8.370   | 77.4%  |
| GetOrCreateProcessByHash-Concurrency2  | 80.84   | 23.37   | 71.1%  |
| GetOrCreateProcessByHash-Concurrency4  | 181.6   | 75.67   | 58.4%  |
| GetOrCreateProcessByHash-Concurrency8  | 408.3   | 194.5   | 52.4%  |
| GetThreadByHash-Concurrency1           | 38.13   | 7.675   | 79.9%  |
| GetThreadByHash-Concurrency2           | 79.98   | 22.71   | 71.6%  |
| GetThreadByHash-Concurrency4           | 177.1   | 60.76   | 65.7%  |
| GetThreadByHash-Concurrency8           | 403.2   | 190.0   | 52.9%  |
| GetOrCreateThreadByHash-Concurrency1   | 38.16   | 7.996   | 79.0%  |
| GetOrCreateThreadByHash-Concurrency2   | 79.82   | 23.73   | 70.3%  |
| GetOrCreateThreadByHash-Concurrency4   | 177.1   | 67.13   | 62.1%  |
| GetOrCreateThreadByHash-Concurrency8   | 405.1   | 151.3   | 62.7%  |

Therefore, this commit reverts the ProcessTree to use the simple LRU
cache until a better solution is designed.

* Part of the performance improvement is also due to the removal of the
outer lock done by 44e59d3.
  • Loading branch information
geyslan authored Sep 13, 2024
1 parent 20a90dc commit 34be604
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 79 deletions.
3 changes: 0 additions & 3 deletions docs/docs/install/config/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ proctree:
cache:
process: 8192
thread: 4096
cache-ttl:
process: 60
thread: 60

capabilities:
bypass: false
Expand Down
3 changes: 0 additions & 3 deletions docs/docs/policies/usage/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@ proctree:
cache:
process: 8192
thread: 8192
cache-ttl:
process: 120
thread: 120
# cri:
# - runtime:
# name: docker
Expand Down
3 changes: 0 additions & 3 deletions examples/config/global_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ proctree:
# cache:
# process: 8192
# thread: 4096
# cache-ttl:
# process: 120
# thread: 120

capabilities:
bypass: false
Expand Down
16 changes: 2 additions & 14 deletions pkg/cmd/cobra/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,21 +173,15 @@ func (c *RegoConfig) flags() []string {
//

type ProcTreeConfig struct {
Source string `mapstructure:"source"`
Cache ProcTreeCacheConfig `mapstructure:"cache"`
CacheTTL ProcTreeCacheTTLConfig `mapstructure:"cache-ttl"`
Source string `mapstructure:"source"`
Cache ProcTreeCacheConfig `mapstructure:"cache"`
}

type ProcTreeCacheConfig struct {
Process int `mapstructure:"process"`
Thread int `mapstructure:"thread"`
}

type ProcTreeCacheTTLConfig struct {
Process int `mapstructure:"process"`
Thread int `mapstructure:"thread"`
}

func (c *ProcTreeConfig) flags() []string {
flags := make([]string, 0)

Expand All @@ -204,12 +198,6 @@ func (c *ProcTreeConfig) flags() []string {
if c.Cache.Thread != 0 {
flags = append(flags, fmt.Sprintf("thread-cache=%d", c.Cache.Thread))
}
if c.CacheTTL.Process != 0 {
flags = append(flags, fmt.Sprintf("process-cache-ttl=%d", c.CacheTTL.Process))
}
if c.CacheTTL.Thread != 0 {
flags = append(flags, fmt.Sprintf("thread-cache-ttl=%d", c.CacheTTL.Thread))
}

return flags
}
Expand Down
19 changes: 0 additions & 19 deletions pkg/cmd/cobra/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,17 +97,12 @@ proctree:
cache:
process: 8192
thread: 4096
cache-ttl:
process: 5
thread: 10
`,
key: "proctree",
expectedFlags: []string{
"source=events",
"process-cache=8192",
"thread-cache=4096",
"process-cache-ttl=5",
"thread-cache-ttl=10",
},
},
{
Expand Down Expand Up @@ -596,20 +591,6 @@ func TestProcTreeConfigFlags(t *testing.T) {
"thread-cache=4096",
},
},
{
name: "process cache ttl set",
config: ProcTreeConfig{
Source: "",
CacheTTL: ProcTreeCacheTTLConfig{
Process: 5,
Thread: 10,
},
},
expected: []string{
"process-cache-ttl=5",
"thread-cache-ttl=10",
},
},
{
name: "all fields set",
config: ProcTreeConfig{
Expand Down
23 changes: 0 additions & 23 deletions pkg/cmd/flags/proctree.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"
"strconv"
"strings"
"time"

"github.com/aquasecurity/tracee/pkg/logger"
"github.com/aquasecurity/tracee/pkg/proctree"
Expand All @@ -21,8 +20,6 @@ Example:
both | process tree is built from both events and signals.
--proctree process-cache=8192 | will cache up to 8192 processes in the tree (LRU cache).
--proctree thread-cache=4096 | will cache up to 4096 threads in the tree (LRU cache).
--proctree process-cache-ttl=60 | will set the process cache element TTL to 60 seconds.
--proctree thread-cache-ttl=60 | will set the thread cache element TTL to 60 seconds.
--proctree disable-procfs-query | Will disable procfs queries during runtime
Use comma OR use the flag multiple times to choose multiple options:
Expand All @@ -38,8 +35,6 @@ func PrepareProcTree(cacheSlice []string) (proctree.ProcTreeConfig, error) {
Source: proctree.SourceNone, // disabled by default
ProcessCacheSize: proctree.DefaultProcessCacheSize,
ThreadCacheSize: proctree.DefaultThreadCacheSize,
ProcessCacheTTL: proctree.DefaultProcessCacheTTL,
ThreadCacheTTL: proctree.DefaultThreadCacheTTL,
ProcfsInitialization: true,
ProcfsQuerying: true,
}
Expand Down Expand Up @@ -98,24 +93,6 @@ func PrepareProcTree(cacheSlice []string) (proctree.ProcTreeConfig, error) {
cacheSet = true
continue
}
if strings.HasPrefix(value, "process-cache-ttl=") {
num := strings.TrimPrefix(value, "process-cache-ttl=")
ttl, err := strconv.Atoi(num)
if err != nil {
return config, err
}
config.ProcessCacheTTL = time.Duration(ttl) * time.Second
continue
}
if strings.HasPrefix(value, "thread-cache-ttl=") {
num := strings.TrimPrefix(value, "thread-cache-ttl=")
ttl, err := strconv.Atoi(num)
if err != nil {
return config, err
}
config.ThreadCacheTTL = time.Duration(ttl) * time.Second
continue
}
if strings.HasPrefix(value, "disable-procfs-query") {
config.ProcfsQuerying = false
continue
Expand Down
31 changes: 17 additions & 14 deletions pkg/proctree/proctree.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import (
"sync"
"time"

"github.com/hashicorp/golang-lru/v2/expirable"
lru "github.com/hashicorp/golang-lru/v2"

"github.com/aquasecurity/tracee/pkg/errfmt"
"github.com/aquasecurity/tracee/pkg/logger"
traceetime "github.com/aquasecurity/tracee/pkg/time"
)
Expand Down Expand Up @@ -68,19 +69,17 @@ type ProcTreeConfig struct {
Source SourceType
ProcessCacheSize int
ThreadCacheSize int
ProcessCacheTTL time.Duration
ThreadCacheTTL time.Duration
ProcfsInitialization bool // Determine whether to scan procfs data for process tree initialization
ProcfsQuerying bool // Determine whether to query procfs for missing information during runtime
}

// ProcessTree is a tree of processes and threads.
type ProcessTree struct {
processes *expirable.LRU[uint32, *Process] // hash -> process
threads *expirable.LRU[uint32, *Thread] // hash -> threads
procfsChan chan int // channel of pids to read from procfs
procfsOnce *sync.Once // busy loop debug message throttling
ctx context.Context // context for the process tree
processes *lru.Cache[uint32, *Process] // hash -> process
threads *lru.Cache[uint32, *Thread] // hash -> threads
procfsChan chan int // channel of pids to read from procfs
procfsOnce *sync.Once // busy loop debug message throttling
ctx context.Context // context for the process tree
procfsQuery bool
timeNormalizer traceetime.TimeNormalizer
}
Expand All @@ -91,22 +90,26 @@ func NewProcessTree(ctx context.Context, config ProcTreeConfig, timeNormalizer t
thrEvicted := 0

// Create caches for processes.
processes := expirable.NewLRU[uint32, *Process](
processes, err := lru.NewWithEvict[uint32, *Process](
config.ProcessCacheSize,
func(k uint32, v *Process) {
func(uint32, *Process) {
procEvited++
},
config.ProcessCacheTTL,
)
if err != nil {
return nil, errfmt.WrapError(err)
}

// Create caches for threads.
threads := expirable.NewLRU[uint32, *Thread](
threads, err := lru.NewWithEvict[uint32, *Thread](
config.ThreadCacheSize,
func(k uint32, v *Thread) {
func(uint32, *Thread) {
thrEvicted++
},
config.ThreadCacheTTL,
)
if err != nil {
return nil, errfmt.WrapError(err)
}

// Report cache stats if debug is enabled.
go func() {
Expand Down

0 comments on commit 34be604

Please sign in to comment.