Decompiler cache causes suboptimal results when reused

[VelizarBG]

When the decompiler cache gets reused weird changes occur, which aren't present when generating the sources without using the cache.

This seems to happen when a big amount of cache hits are present, as is the case between two versions of the game that are adjacent in time, and when adding/removing an access widener entry. In the first case it pollutes the diff quite a lot, and reduces readability in both cases.

Steps to reproduce

0. Delete your decompiler cache.
1. Clone the reproduction repo.
2. Checkout the 1.20.6 branch.
3. Run gradlew genSourcesWithVineflower --use-cache
4. Checkout the 24w18a branch.
5. Repeat step 3.
6. Store a copy of the generated sources somewhere.
7. Run gradlew genSourcesWithVineflower --no-use-cache
8. Diff the sources generated with cache and the ones without.
9. Notice the suboptimal differences(e.g. missing @Override annotations, unnecessary casts, missing generics, and so on).

PS: This also applies to the CFR decompiler. Haven't tested FernFlower.

[modmuss50]

Humm, intresting. I can see that being an issue, the easy fix for this is to not share the caches between diffrent versions. Or take into account the entire class hierarchy when calcuating the cache key. 🤔

[VelizarBG]

Just tested it with access wideners, and it seems to be an issue there as well :/
Though, quite importantly, the defects only happen in the classes that the cache misses.

[isXander]

Humm, intresting. I can see that being an issue, the easy fix for this is to not share the caches between diffrent versions. Or take into account the entire class hierarchy when calcuating the cache key. 🤔

Not sharing cache between versions would be a huge bummer. A lot of the times there a loads of cache hits as you probably know.

[VelizarBG]

Updated the title and description to better suite the scope of the issue. I'd also post actual examples, but not sure if that's a great idea.

[VelizarBG]

@modmuss50 Thanks so much for the fix! All of the defects seem to be gone, except for the one with the weird imports for inner classes(sometimes they are imported explicitly, other times referenced by their fully qualified name). It's not a big deal but it'd be nice if it could be mitigated.

[modmuss50]

@modmuss50 Thanks so much for the fix! All of the defects seem to be gone, except for the one with the weird imports for inner classes(sometimes they are imported explicitly, other times referenced by their fully qualified name). It's not a big deal but it'd be nice if it could be mitigated.

Are you seeing this when switching between MC versions or when enabling/disabling AWs?

[VelizarBG]

I tested the former. Let me also try the latter...
Yep, it's an issue there as well.

[modmuss50]

Ok, thanks. Ill take another look.

Ill re-open the issue so I dont forget 👍