search

tikv / pprof-rs

A Rust CPU profiler implemented with the help of backtrace-rs
Apache License 2.0
1.29k stars 99 forks source link

Replace TmpFdArray with Ring to remove dependency on tmpfs #183

Closed XAMPPRocky closed 5 months ago

XAMPPRocky commented 1 year ago

Hello 👋 This PR removes the tmpfs backing storage to the temp_array, replacing it with a LIFO ring buffer. The motivation behind this change is that in Kubernetes, by default your container is running in a read only filesystem, so if you want to be able to use it without requiring that each container their own tmpfs, we need to remove the usage of the temporary file. I'd be open to making a followup adding it back as an optional feature if it's important to keep the tmpfs storage for some platforms, but I think an entirely memory based profiler by default is more intuitive.

YangKeao commented 1 year ago

Thanks for the contribution.

I'm not sure whether removing the existing samples is expected :thinking: as the number of samples are related with the profiling frequency and duration. The current behavior is consistent with gperftools.

Maybe we could add a flag (runtime argument if possible, or a feature is good enough) to allow users to make their own choices? (Or fallback to full memory store when the tmpfs is not writable :thinking: ).

XAMPPRocky commented 1 year ago

runtime argument if possible, or a feature is good enough

I'll add it as a compile-time feature flag, that's on by default, that way it maintains the same behaviour, but it can be turned off. It would be nice to not have to depend on tempfile when it's not needed.