Re: [abc] Soot related issue (fwd)

FFrom the point of view of Soot maintenance, I wouldn't object to
changing the relevant classes to store an ArrayList internally, provided
the currrent interface is preserved (in particular, the current
constructor takes an array of args, so an addapter constructor would
need to be made to convert that array into an ArrayList). Aside from the
constructors, none of the current interface exposes the array, AFAIK,
so this should be doable.

However, this shifts the performance penalty from being only in the
around weaver to being in all of Soot in general. An ArrayList is going
to be bigger and slower than a raw array in the general case, so we're
hurting all other applications of Soot.

Also, it seems hard to believe that this is really the problem. The
constructor that I'm looking at (AbstractVirtualInvokeExpr) just has
a few simple assignments to fields. Unless the number of arguments grows
very large, copying the array shouldn't be very expensive either. After
all, the ArrayList needs to copy the array from time to time too. Hmm...
JVirtualInvokeExpr also creates boxes for all the values (which it needs
to). I can see this taking some time.

Digging a bit deeper, I see that the Jimple interface actually takes a
list, rather than an array, and converts the list to an array. In my
experience, Lists are quite slow. If you tell me that the bottleneck is
building up this list and then traversing it to put things in the array
and then throwing the List away again, I'll believe you.

Anyway, here's a solution that I think would avoid all the
disadvantages I've mentioned above. Presumably, only the calls to
around-weaver-specific methods need to be modified frequently.
So, we should just subclass JVirtualInvokeExpr and friends with a
different class that uses an ArrayList instead of an array, and
supports lengthening it quickly (by just making one new box, instead
of recreating all the boxes). Alternatively, we could not subclass
JVirtualInvokeExpr, but just make a class that implements the
VirtualInvokeExpr interface. This new InvokeExpr would be used for the
around stuff in those places where it needs to be growable. What do you
think?

(Aside: subclassing JVirtualInvokeExpr breaks the
OO/factory-design-pattern design of Jimple in that it forces abc to
import soot.jimple.internal. Raja would probably not be pleased. But
then, this has already been violated so many times in grimp and dava
that it's silly to complain about it now. Still, the alternative that
I mention above, just implementing VirtualInvokeExpr, avoids this
transgression. I think it will be the cleaner solution in general, so
I think it's preferable. It's a bit more likely to cause problems with
broken code that explicitly looks for a JVirtualInvokeExpr, but there
hopefully isn't much code like that; if there is, it will give us a good
reason to fix it.)

Ondrej

On Mon, Apr 18, 2005 at 06:13:04PM -0400, Laurie Hendren wrote:
> Sascha has the following question ... it's actually quite important
> because it is slowing down our weaver a lot.
>
> Cheers, Laurie
>
>
> +-------------------------------------------------------------+
> | Laurie Hendren, Professor, School of Computer Science |
> | McGill University |
> | 318 McConnell Engineering Building tel: (514) 398-7391 |
> | 3480 University Street fax: (514) 398-3883 |
> | Montreal, Quebec H3A 2A7 hendren@cs.mcgill.ca |
> | CANADA http://www.sable.mcgill.ca/~hendren |
> | http://wwww.sable.mcgill.ca http://aspectbench.org |
> +-------------------------------------------------------------+
>
> ---------- Forwarded message ----------
> Date: Mon, 18 Apr 2005 23:03:40 +0100
> From: Sascha Kuzins <sascha.kuzins@comlab.ox.ac.uk>
> Reply-To: abc@comlab.ox.ac.uk
> To: abc@comlab.ox.ac.uk
> Subject: [abc] Soot related issue
>
> Hi all,
>
> I'm in the process of identifying bottlenecks in abc.
> The around weaver weaves incrementally, and in this process keeps adding
> arguments to methods and updating all invocations.
> The problem is that arguments cannot be added to InvokeExprs, and so the
> weaver keeps recreating InvokeExprs when it needs to add arguments.
> This turns out to be a hot spot.
>
> Would it be possible to allow arguments to be added to InvokeExprs?
> Maybe we could use an ArrayList instead of the array that's currently
> used? Let me know what you think.
>
> Cheers,
>
> Sascha
>
>
Received on Tue Apr 19 03:37:34 2005