feat(dotnet): keep only most recently used strings in memory LRU cache#1338
feat(dotnet): keep only most recently used strings in memory LRU cache#1338danielpacak wants to merge 1 commit intoopen-telemetry:mainfrom
Conversation
| peStringsCacheSize = 1024 | ||
|
|
||
| // TTL of entries in the LRU cache holding the dotnet strings. | ||
| peStringsCacheTTL = 1 * time.Hour |
There was a problem hiding this comment.
I'm wondering if these configurations should be exposed in some way as different environment might require different options.
Also, should these settings a bit more aligned? peInfoCacheTTL uses 6 hours and prStringsCacheTTL uses 1 hour.
There was a problem hiding this comment.
This makes perfect sense. The PE cache should be longer to avoid doing the heavy reparsing of PE files. This needs to be smaller to reduce memory usage.
| // stringsReader is the io.SectionReader starting at the dotnet string heap. | ||
| stringsReader io.ReaderAt | ||
|
|
||
| // file is the reference to the backing file so we can close it when evicted from cache. | ||
| file process.ReadAtCloser |
There was a problem hiding this comment.
This keeps the file descriptor open. I'm not sure if this causes other problems when the PE cache gets too many cached files and open FDs?
I wonder if it would be possible instead to get only the file offset where the strings section starts. And when reading the string, then use mappings to determine where in the target process this file is mapped and use the remotememory primitive to read it from the running process.
There was a problem hiding this comment.
Thanks @fabled ! That's a good point. In the "worst" case we may end up with at least peInfoCacheSize = 16384 files opened by the ebpf-profiler process. Given that soft and hard limits vary across environments, it's going to crash in the wild sooner than later. So I think holding opened file descriptors is not super scalable.
That said, I'm also leaning towards remote memory access. I'll give it a try and use the remote memory primitive instead.
3 participants
Resolves: #1329