Re: It's time to kill statistical profiling

From: Konstantin Belousov <kostikbel_at_gmail.com>
Date: Fri, 18 Jun 2021 13:57:51 UTC
On Fri, Jun 18, 2021 at 07:35:44AM -0600, Warner Losh wrote:
> On Fri, Jun 18, 2021 at 1:37 AM Poul-Henning Kamp <phk@phk.freebsd.dk>
> wrote:
> 
> > Warners work to document the kernel timers in D30802 brought stathz up
> > again.
> >
> > To give a representative result, statistical profiling needs to
> > sample no less than approx 0.1% of instructions.
> >
> > On a VAX that meant running the statistical profiling at O(1kHz).
> >
> > On my 4 CPU, two thread, 2GHz laptop that means statistical profiling
> > needs to run at O(10 MHz), which is barely doable.
> >
> > But it is worse:
> >
> > The samples must be unbiased with respect to the system activity,
> > which was already a problem on the VAX and which is totally impossible
> > on modern hardware, with message based interrupts, deep pipelines
> > and telegraphic distance memory[1].
> >
> > Therefore statistical profiling is worse than useless: it is downright
> > misleading, which is why modern CPUs have hardware performance counters.
> >
> > Instead of documenting stathz, I suggest we retire statistical
> > profiling and convert the profiled libraries to code-coverage
> > profiling (-fprofile-arcs and -ftest-coverage)
> >
> > Poul-Henning
> >
> > [1] One could *possibly* approch unbiased samples, by locking the
> > stathz code path in L1 cache and disable L1 updates, but then
> > the results would be from an entirely different system.
> >
> 
> I've documented them as obsolete in favor of hwpmc (though I know that's not
> quite the whole story).  I'll remove that when we remove the feature, since
> it
> also documents the clockinfo sysctl a bit.
> 
> stathz and profhz drive the setitimer ITIMER_PROF functionality. While this
> was in POSIX.1-2001, it was marked deprecated in POSIX.1-2008. There's
> sysctls
> that return these values to userland as well, but those could return bogus
> values.
> Our current docs for this are vague enough to give some cover :)
> 
> Deleting the code from the kernel is the easy bit, I guess. I generally
> support
> this notion, but don't have the time to hunt down all the stuff in base and
> ports that uses these and make appropriate changes (up to and including git
> rm).

You cannot remove a kernel feature without breaking ABI.
I do not see why do we need to remove it.

The userspace bits, specifically build system glue to build _p.a libraries,
is different, of course.  We already have profiling static libraries build
disabled.  I also killed (as removed from the kernel build and config(8))
the support for -pg profiling).

But for userspace, instead of removing, it could be quite useful to
generalize it, instead of removing. In particular, to allow user
to build some 'flavor' of system libraries, with intent to support
either code coverage analysis, or PGO, or whatever toolchains authors
implement. User would specify which additional toolchain options to pass
instead of -pg, and probably should be able to provide his own crt1.o
flavor.