Re: [abc] Soot related issue (fwd)

Hi Ondrej,

I think the solution of creating custom classes for the weaver is a good
one, and I do understand you objection to putting this into soot and
hurting performance in other applications.
On the other hand, I found it is usually soot's style to allow changes
to objects after creation, and this InvokeExpr limitation is a bit
unusual. Then again, I guess not so many applications will want to make
changes to the number of arguments of a call, although there are some
good use cases: for example, we have one optimization pass that detects
if all calls to a method pass the same constant argument, and then
inlines this argument into the method and removes the parameter from the
method signature. This is a pretty general optimization, and it suffers
from the same limitation.

Sascha

Ondrej Lhotak wrote:
> 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 17:47:48 2005