search

soot-oss / soot

Soot - A Java optimization framework
GNU Lesser General Public License v2.1
2.85k stars 706 forks source link

SmartLocalDefs is often recomputed #138

Open ericbodden opened 10 years ago

ericbodden commented 10 years ago

A simple investigation shows that Soot often recomputes SmartLocalDefs multiple times for the same method (often five times). The analysis is rather costly and should probably be cached instead.

ericbodden commented 10 years ago

Note: This might not be as easily fixable as I thought. The problem is that the Jimple code is often transformed between subsequent invocations. I guess SSA/Shimple might help here but that would be a quite incompatible change.

patricklam commented 10 years ago

Could we checksum the Jimple Body or otherwise ensure that it's not changed?

SSA form also imposes a lot of constraints in keeping things up to date; it may not actually have a performance win and certainly makes code more complicated.

On Tue, Oct 22, 2013 at 10:24 AM, Eric Bodden notifications@github.comwrote:

Note: This might not be as easily fixable as I thought. The problem is that the Jimple code is often transformed between subsequent invocations. I guess SSA/Shimple might help here but that would be a quite incompatible change.

— Reply to this email directly or view it on GitHubhttps://github.com/Sable/soot/issues/138#issuecomment-26807215 .

StevenArzt commented 10 years ago

I would vote against SSA since there are many transformers which would have to be rewritten to maintan SSA (which would complicate the code quite a bit) or would have to be followed by a step that re-establishes SSA. In sum, I'm not sure that this would by us anything in terms of performance.

badlogic commented 9 years ago

We are hitting this issue as well, and it imposes quite some runtime overhead in our application. Most of the time seems to be spend in the flow analysis according to JProfiler. I saw some recent changes in ForwardFlowAnalysis, did this improve performance?

ericbodden commented 9 years ago

Yes, we had some performance improvements in the past year, however the main problem still remains: The conversion to Jimple happens in several passes, each pass of which might change (and often does change) the respective JimpleBody. After each of these passes, the SimpleLocalDefs must be recomputed, unless we find a way to incrementally update it, or to maybe perform some checksums as @patricklam suggested. If I find some time I will look into this a bit.

badlogic commented 9 years ago

Great. I ported the latest flow analysis code which had a tiny impact on the runtime.

On Sat, Jan 31, 2015 at 2:38 PM, Eric Bodden notifications@github.com wrote:

Yes, we had some performance improvements in the past year, however the main problem still remains: The conversion to Jimple happens in several passes, each pass of which might change (and often does change) the respective JimpleBody. After each of these passes, the SimpleLocalDefs must be recomputed, unless we find a way to incrementally update it, or to maybe perform some checksums as @patricklam https://github.com/patricklam suggested. If I find some time I will look into this a bit.

— Reply to this email directly or view it on GitHub https://github.com/Sable/soot/issues/138#issuecomment-72318066.

ericbodden commented 9 years ago

I implemented some pooling in commit 454f7c4 Let's see what our nightly builds say about this. Hope it works. I ran some statistics. If it works then we should safe the creation of about every second SmartLocalDefs instance (and the connected SimpleLiveLocals and ExceptionalUnitGraph instances).

I was unable to update some clients that use DalvikThrowAnalysis. To pool for those, too, we would need to extend the mechanism a bit.

badlogic commented 9 years ago

This is awesome! I'll try to backport it to our Soot fork and will report results. Looks like a simple fix :) On Feb 2, 2015 1:01 AM, "Eric Bodden" notifications@github.com wrote:

I implemented some pooling in commit 454f7c4 https://github.com/Sable/soot/commit/454f7c43163bfd66340b0b1a7300aee3085d2ef8 Let's see what our nightly builds say about this. Hope it works. I ran some statistics. If it works then we should safe the creation of about every second SmartLocalDefs instance (and the connected SimpleLiveLocals and ExceptionalUnitGraph instances).

I was unable to update some clients that use DalvikThrowAnalysis. To pool for those, too, we would need to extend the mechanism a bit.

— Reply to this email directly or view it on GitHub https://github.com/Sable/soot/issues/138#issuecomment-72392568.

ericbodden commented 9 years ago

Note to self: we should find a way to clear the pool at a sensible time, when Jimplification has completed.

ericbodden commented 9 years ago

@badlogic You can probably simply cherry-pick the commit via git.