Avoid unnecessary linked list traversals in MethodLinker.

[LlamaLad7]

The chains of method links can get surprisingly large and a huge amount of time can be wasted repeatedly traversing them.
We can cut down most of this time by flattening the chain each time we traverse it, pointing every element straight to the end.
The chains themselves are assumed to be an implementation detail of this class, in which case it's fine to flatten them at any time.
Some profiling from building one of my projects with ProGuard:
Before:

After:

I'd imagine that there are ways to avoid linked lists at all in this linking process, but this change is minimally invasive to the external API and the speedup is very large.

[rubenpieters]

Hey @LlamaLad7 ! Thanks for the PR, your approach looks like a nice improvement. We will be looking at the suggestion further, but we will need to do some additional testing first to ensure the change is safe to apply also in our internal projects using ProGuardCore.
We would also be interested in reproducing the results you mentioned, do you mind sharing which project you tested the runtime improvements on?

[LlamaLad7]

@rubenpieters Unfortunately the project is proprietary, so I can't share it. We are using obfuscation, hence the work in MethodLinker, but apart from that I think the only significant thing is the size of our build, since we have ~25,000 program classes and ~75,000 library classes. ProGuard deals with a project of this scale surprisingly well in most aspects, but about 50% of our build time is spent just traversing these chains.

I've made a slightly contrived example here (https://github.com/LlamaLad7/slow-proguard-example) with a lot of methods in a large inheritance chain. MethodLinker spends about 15 seconds traversing the chains without my change, and about 1 second with my change. Hopefully this helps.

[rubenpieters]

@LlamaLad7 The reproducing project you shared is indeed helpful in reproducing the effects you describe, so thanks for that! We are planning on having a further look on the impact of this in our internal projects, as we haven't gotten to it yet. Mainly to ensure we don't introduce any unexpected side effects as mentioned before.

[colinmcdonald22]

Hi @rubenpieters! Colleague of @LlamaLad7 here; I just wanted to check in on this PR. We're anxious to get it merged as it'd result in a pretty significant speed up of our CI runs.

We're considering compiling our own build of ProGuard and using that, but if this PR is close to being approved, we'd be happy to wait it out.

[rubenpieters]

Hey @colinmcdonald22 , we will still need a bit more time to properly review this. We have reviewed our internal usages of the MethodLinker, and that looks fine with your change as well. So we will be able to merge the PR soon (likely by the end of the week). But before it makes it into a release, e.g. ProGuard we would like to make sure it has no adverse effects on our own internal CI tests. I would expect ~2 ish weeks before we can make a release with this change in it.

Does that provide enough information regarding the timeline for you?

[colinmcdonald22]

Hey @colinmcdonald22 , we will still need a bit more time to properly review this. We have reviewed our internal usages of the MethodLinker, and that looks fine with your change as well. So we will be able to merge the PR soon (likely by the end of the week). But before it makes it into a release, e.g. ProGuard we would like to make sure it has no adverse effects on our own internal CI tests. I would expect ~2 ish weeks before we can make a release with this change in it.

Does that provide enough information regarding the timeline for you?

Yes that’s very helpful, thank you!