Messages in this thread | | | From | Ian Rogers <> | Date | Tue, 28 Nov 2023 11:13:55 -0800 | Subject | Re: [PATCHSET 0/8] perf annotate: Make annotation_options global (v1) |
| |
On Tue, Nov 28, 2023 at 9:54 AM Namhyung Kim <namhyung@kernel.org> wrote: > > Hello, > > It used to have annotation_options for each command separately (for > example, perf report, annotate, and top), but we can make it global as > they never used together (with different settings). This would save > some memory for each symbol when annotation is enabled. > > This code is available at 'perf/annotate-option-v1' branch in > > git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git > > Thanks, > Namhyung
Thanks for doing this and I think it is progress. I think there is a common problem with having things be an option rather than say part of session. Having a global variable seems unfortunate but I'm not sure if in all locations we have easy access to the session. The rough structure with annotations as I understand it is:
session has machines a machine has dsos a dso has symbols a symbol has an annotation
Annotation is something of unfortunate abstraction as it covers things like an IPC per symbol (why hard code to just IPC?) and things like source files and line numbers.
A recent success story where we got rid of a configuration variable was by switching to lazy allocation with sorting by name for symbols within a dso. If we could have a lazy allocation model with annotations then maybe we can do away with large hammers like global options.
Thanks, Ian
> Namhyung Kim (8): > perf annotate: Introduce global annotation_options > perf report: Convert to the global annotation_options > perf top: Convert to the global annotation_options > perf annotate: Use global annotation_options > perf ui/browser/annotate: Use global annotation_options > perf annotate: Ensure init/exit for global options > perf annotate: Remove remaining usages of local annotation options > perf annotate: Get rid of local annotation options > > tools/perf/builtin-annotate.c | 43 +++++---- > tools/perf/builtin-report.c | 37 ++++---- > tools/perf/builtin-top.c | 45 +++++----- > tools/perf/ui/browsers/annotate.c | 85 ++++++++---------- > tools/perf/ui/browsers/hists.c | 34 +++---- > tools/perf/ui/browsers/hists.h | 2 - > tools/perf/util/annotate.c | 142 +++++++++++++++--------------- > tools/perf/util/annotate.h | 38 ++++---- > tools/perf/util/block-info.c | 6 +- > tools/perf/util/block-info.h | 3 +- > tools/perf/util/hist.h | 25 ++---- > tools/perf/util/top.h | 1 - > 12 files changed, 206 insertions(+), 255 deletions(-) > > > base-commit: 757489991f7c08603395b85037a981c31719c92c > -- > 2.43.0.rc1.413.gea7ed67945-goog >
| |