Re: [Xen-devel] [PATCH v4 3/4] libxl: Some optimizations in libxl_set_memory_target()

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Mon, Apr 29, 2013 at 12:16:43PM +0100, Ian Campbell wrote:
> On Mon, 2013-04-29 at 12:09 +0100, Daniel Kiper wrote:
> > Some optimizations in libxl_set_memory_target().
>
> Please describe them in the commit message.

OK.

> At first glance it just looks like you are renaming some variables from
> "memmax"/"target"/"uuid" to "s", why? Just to reduce the number of local
> variables?
>
> They seem more meaningful and easier to read with their current names
> and if you are "optimising" the use of local variable then that seems
> like a false optimisation to me -- any modern compiler can surely work
> out that the lifetimes of these various things do not overlap and do the
> sensible thing WRT register allocation.
>
> At the very least this renaming is hiding what you are actually doing.

Here are four things:
  - reduction of number of variables which you found,
  - removal of unneeded intialization; probably it was
    needed some time ago but now it is not,
  - removal of memorykb = new_target_memkb assignment
    with relevant changes in other places,
  - merge of video memory size calculation which
    is currently in two places.

I do not insist on first thing but I think that
other optimizations are worth to do. If you wish
I could do all of them in separate patches.

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel