From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51853) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SA20Y-00009a-DA for qemu-devel@nongnu.org; Tue, 20 Mar 2012 12:34:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SA205-00085b-Fu for qemu-devel@nongnu.org; Tue, 20 Mar 2012 12:34:01 -0400 Received: from mail-lpp01m010-f45.google.com ([209.85.215.45]:42404) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SA205-00085H-1E for qemu-devel@nongnu.org; Tue, 20 Mar 2012 12:33:33 -0400 Received: by lahe6 with SMTP id e6so199891lah.4 for ; Tue, 20 Mar 2012 09:33:30 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <87obrrgyoc.fsf@ginnungagap.bsc.es> References: <20120313200235.24179.63987.stgit@ginnungagap.bsc.es> <20120313200332.24179.78152.stgit@ginnungagap.bsc.es> <87obrrgyoc.fsf@ginnungagap.bsc.es> Date: Tue, 20 Mar 2012 16:33:30 +0000 Message-ID: From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 10/12] trace: [tracetool] Automatically establish available backends and formats List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?ISO-8859-1?Q?Llu=EDs_Vilanova?= Cc: harsh@linux.vnet.ibm.com, qemu-devel@nongnu.org, aneesh.kumar@linux.vnet.ibm.com 2012/3/20 Llu=EDs Vilanova : > Stefan Hajnoczi writes: > >> 2012/3/13 Llu=EDs Vilanova : >>> Adds decorators to establish which backend and/or format each routine i= s meant >>> to process. >>> >>> With this, tables enumerating format and backend routines can be elimin= ated and >>> part of the usage message can be computed in a more generic way. >>> >>> Signed-off-by: Llu=EDs Vilanova >>> Signed-off-by: Harsh Prateek Bora >>> --- >>> =A0Makefile.objs =A0 =A0 =A0 =A0| =A0 =A06 - >>> =A0Makefile.target =A0 =A0 =A0| =A0 =A03 >>> =A0scripts/tracetool.py | =A0320 ++++++++++++++++++++++++++++++++------= ------------ >>> =A03 files changed, 211 insertions(+), 118 deletions(-) > >> I find the decorators are overkill and we miss the chance to use more >> straightforward approaches that Python supports. =A0The decorators build >> structures behind the scenes instead of using classes in an open coded >> way. =A0I think this makes it more difficult for people to modify the >> code - they will need to dig in to what exactly the decorators do - >> what they do is pretty similar to what you get from a class. > >> I've tried out an alternative approach which works very nicely for >> formats. =A0For backends it's not a perfect fit because it creates >> instances when we don't really need them, but I think it's still an >> overall cleaner approach: > >> https://github.com/stefanha/qemu/commit/3500eb43f80f3c9200107aa0ca19a1b5= 7300ef8a > >> What do you think? > > I don't like it: > > 1) Format and backend tables must be manually filled. > > 2) The base Backend class has empty methods for each possible format. > > 3) There is no control on format/backend compatibility. > > But I do like the idea of having a single per-backend class having method= s for > each possible format. > > The main reason for automatically establishing formats, backends and thei= r > compatibility is that the instrumentation patches add some extra formats = and > backends, which makes the process much more tedious if it's not automated= . > > Whether you use decorators or classes is not that important. > > Auto-registration can be accomplished using metaclasses and special > per-format/backend "special" attributes the metaclasses will look for (e.= g. NAME > to set the commandline-visible name of a format/backend.). > > Format/backend compatibility can be established by introspecting into the > methods on each backend child class, matched against the NAMEs of the reg= istered > formats. > > But going this way does not sound to me like it will be much clearer than > decorators. > > Do you agree? (I'll wait on solving this before fixing the rest of your c= oncerns > in this series). Do you still prefer classes over decorators? For formats the Format class plus a dict approach is much nicer than decorators. The code is short and easy to understand. For backends it becomes a little tougher and I was wondering whether splitting the code into modules would buy us something. The fact that you've added '####...' section delimeters shows that tracetool.py is growing to long and we're putting too many concepts into one file. If each backend is a module then we have a natural way of containing backend-specific code. Perhaps the module can register itself when tracetool.py wildcard imports them all. I think this will approach the level of magic that decorators involve but with the bonus that we begin to split the code instead of growing a blob. What do you think of putting each backend in its own module? Do you have a link to your latest code that adds formats/backends? I'd like to take a quick look to make sure I understand where you're going with this - I've only been thinking of the current set of formats/backends. As the next step with this patch series we could drop this patch. It doesn't make an external difference. Then we can continue to discuss how to best handle backends as a separate patch. Stefan