From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Ingo Molnar <mingo@kernel.org>, Jiri Olsa <jolsa@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
linux-kernel@vger.kernel.org, Namhyung Kim <namhyung@kernel.org>,
Borislav Petkov <bp@alien8.de>
Subject: Re: [PATCH v2 14/14] perf tools: Move subcommand framework and related utils to libapi
Date: Wed, 9 Dec 2015 12:58:08 -0300 [thread overview]
Message-ID: <20151209155808.GO11564@kernel.org> (raw)
In-Reply-To: <20151209123315.GA15897@treble.redhat.com>
Em Wed, Dec 09, 2015 at 06:33:15AM -0600, Josh Poimboeuf escreveu:
> On Wed, Dec 09, 2015 at 09:03:43AM +0100, Ingo Molnar wrote:
> >
> > * Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >
> > > > > wouldn't necessarily be a clean split. It would also possibly create more
> > > > > room for error for the users of libapi, since there would then be three
> > > > > config interfaces instead of one.
> > > >
> > > > Humm, and now that you talk... libapi was supposed to be just sugar coating
> > > > kernel APIs, perhaps we need to put it somewhere else in tools/lib/ than in
> > > > tools/lib/api/?
> > >
> > > Ah, I didn't realize libapi was a kernel API abstraction library. Shall we put
> > > it in tools/lib/util instead?
> >
> > Yay, naming discussion! ;-)
>
> Oh boy! ;-)
>
> > So if this is about abstracting out the (Git derived) command-line option parsing
> > UI and help system, 'util' sounds a bit too generic.
> >
> > We could call it something like 'lib/cmdline', 'lib/options'?
> >
> > The (old) argument against making too finegrained user-space libraries was that
> > shared libraries do have extra runtime costs - this thinking resulted in catch-all
> > super-libraries like libgtk:
> >
> > size /usr/lib/x86_64-linux-gnu/libgtk-3.so.0
> > text data bss dec hex filename
> > 7199789 57712 15128 7272629 6ef8b5 /usr/lib/x86_64-linux-gnu/libgtk-3.so.0
> >
> > But in tools/ we typically link the libraries statically so there's no shared
> > library cost to worry about. (Build time linking is a good idea anyway, should we
> > ever want to make use of link-time optimizations. It also eliminates version skew
> > and library compatibility breakage.)
> >
> > The other reason for the emergence of super-libraries was the high setup cost of
> > new libraries: it's a lot easier to add yet another unrelated API to libgtk than
> > to start up a whole new project and a new library. But this setup cost is very low
> > in tools/ - one of the advantage of shared repositories.
> >
> > So I think in tools/lib/ we can continue to do a clean topical separation of
> > libraries, super-libraries are not needed.
>
> I definitely agree that for the reasons you outlined, something like
> 'lib/cmdline' would be a good idea. Except... there's a wrinkle, of
> course.
>
> The library also includes non-cmdline-related dependencies. And these
> dependencies are directly used by perf as well. So if we name it
> 'cmdline', perf would have includes like:
>
> #include <cmdline/pager.h>
> #include <cmdline/strbuf.h>
> #include <cmdline/term.h>
> #include <cmdline/wrapper.h>
> ...etc...
>
> So it would be using several functions from the 'cmdline' library which
> are unrelated to 'cmdline'.
>
> For that reason I would vote to name it 'lib/util'. But I don't really
> care, I'd be ok with 'lib/marshmallow' if that's what you guys wanted
> :-)
Right, now you see why this wasn't librarised before, huh? Untangling
bits in a way that this gets sane takes a bit of time.
I'm going thru your patchkit to erode it a bit, taking uncontroversial
patches.
I also would just do one thing first, i.e. just move the cmdline parts
to lib/cmdline/, then we would look at the rest. I.e. reduce the problem
first.
Yeah, I haven't looked deeply how difficult that would be :-\
- Arnaldo
next prev parent reply other threads:[~2015-12-09 15:58 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-08 4:21 [PATCH v2 00/14] perf tools: Move perf subcommand framework into lib/tools Josh Poimboeuf
2015-12-08 4:21 ` [PATCH v2 01/14] perf: Fix 'make clean' Josh Poimboeuf
2015-12-08 17:40 ` Jiri Olsa
2015-12-08 18:40 ` Jiri Olsa
2015-12-08 18:49 ` Josh Poimboeuf
2015-12-08 18:46 ` Josh Poimboeuf
2015-12-08 4:21 ` [PATCH v2 02/14] perf: Use -iquote for local include paths Josh Poimboeuf
2015-12-08 4:21 ` [PATCH v2 03/14] perf: Split up util.h Josh Poimboeuf
2015-12-08 4:21 ` [PATCH v2 04/14] perf: Move term functions out of util.c Josh Poimboeuf
2015-12-09 15:53 ` Arnaldo Carvalho de Melo
2015-12-10 8:18 ` [tip:perf/core] perf tools: " tip-bot for Josh Poimboeuf
2015-12-08 4:21 ` [PATCH v2 05/14] perf: Remove unused pager_use_color variable Josh Poimboeuf
2015-12-09 15:43 ` Arnaldo Carvalho de Melo
2015-12-10 8:18 ` [tip:perf/core] perf tools: " tip-bot for Josh Poimboeuf
2015-12-08 4:21 ` [PATCH v2 06/14] perf: Split up cache.h Josh Poimboeuf
2015-12-08 4:21 ` [PATCH v2 07/14] perf: Remove cache.h Josh Poimboeuf
2015-12-08 4:21 ` [PATCH v2 08/14] perf: Save cmdline arguments earlier Josh Poimboeuf
2015-12-10 8:18 ` [tip:perf/core] perf tools: " tip-bot for Josh Poimboeuf
2015-12-08 4:21 ` [PATCH v2 09/14] perf: Remove check for unused PERF_PAGER_IN_USE Josh Poimboeuf
2015-12-08 4:21 ` [PATCH v2 10/14] perf: Move cmd_version() to builtin-version.c Josh Poimboeuf
2015-12-10 8:19 ` [tip:perf/core] perf tools: " tip-bot for Josh Poimboeuf
2015-12-08 4:21 ` [PATCH v2 11/14] perf: Move help_unknown_cmd() to its own file Josh Poimboeuf
2015-12-08 4:21 ` [PATCH v2 12/14] perf tools: Move strlcpy() to tools/lib/string.c Josh Poimboeuf
2015-12-08 4:21 ` [PATCH v2 13/14] perf tools: Move tools/lib/string.c to libapi Josh Poimboeuf
2015-12-08 4:21 ` [PATCH v2 14/14] perf tools: Move subcommand framework and related utils " Josh Poimboeuf
2015-12-08 18:16 ` Jiri Olsa
2015-12-08 18:49 ` Josh Poimboeuf
2015-12-08 19:09 ` Arnaldo Carvalho de Melo
2015-12-08 19:17 ` Josh Poimboeuf
2015-12-08 19:40 ` Arnaldo Carvalho de Melo
2015-12-08 21:48 ` Josh Poimboeuf
2015-12-08 22:27 ` Arnaldo Carvalho de Melo
2015-12-08 23:07 ` Josh Poimboeuf
2015-12-09 8:03 ` Ingo Molnar
2015-12-09 12:33 ` Josh Poimboeuf
2015-12-09 15:58 ` Arnaldo Carvalho de Melo [this message]
2015-12-09 18:59 ` Josh Poimboeuf
2015-12-10 1:40 ` Namhyung Kim
2015-12-10 14:54 ` Josh Poimboeuf
2015-12-10 21:35 ` Josh Poimboeuf
2015-12-11 11:21 ` Arnaldo Carvalho de Melo
2015-12-10 12:55 ` Arnaldo Carvalho de Melo
2015-12-10 15:15 ` Josh Poimboeuf
2015-12-10 1:58 ` Namhyung Kim
2015-12-10 2:00 ` [PATCH v2 00/14] perf tools: Move perf subcommand framework into lib/tools Namhyung Kim
2015-12-10 15:11 ` Josh Poimboeuf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151209155808.GO11564@kernel.org \
--to=acme@kernel.org \
--cc=bp@alien8.de \
--cc=jolsa@redhat.com \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.