2012/3/20 Lluís Vilanova <vilan...@ac.upc.edu>: > Stefan Hajnoczi writes: > >> 2012/3/13 Lluís Vilanova <vilan...@ac.upc.edu>: >>> Adds decorators to establish which backend and/or format each routine is >>> meant >>> to process. >>> >>> With this, tables enumerating format and backend routines can be eliminated >>> and >>> part of the usage message can be computed in a more generic way. >>> >>> Signed-off-by: Lluís Vilanova <vilan...@ac.upc.edu> >>> Signed-off-by: Harsh Prateek Bora <ha...@linux.vnet.ibm.com> >>> --- >>> Makefile.objs | 6 - >>> Makefile.target | 3 >>> scripts/tracetool.py | 320 >>> ++++++++++++++++++++++++++++++++------------------ >>> 3 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. The decorators build >> structures behind the scenes instead of using classes in an open coded >> way. I 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. For 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/3500eb43f80f3c9200107aa0ca19a1b57300ef8a > >> 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 methods for > each possible format. > > The main reason for automatically establishing formats, backends and their > 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 > registered > 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 > concerns > 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
