Fix error when a mixin missing a target has an inner class

[Chocohead]

When a mixin has multiple target classes and at least one is missing, as well as at least one inner class to merge into the targets, Mixin would previously throw an error when applying the mixin to any target.

Using the following mixin:

@Mixin(value = {Target.class}, targets = "com.chocohead.test.Missing")
abstract class MissingTargetMixin {
	class Inner {
	}
}

would produce the following log upon trying to load Target:

[DEBUG] [FabricLoader/Mixin]: Preparing tests.mixins.json (1)
[WARN] [FabricLoader/Mixin]: Error loading class: com/chocohead/test/Missing (java.lang.ClassNotFoundException: com/chocohead/test/Missing)
[WARN] [FabricLoader/Mixin]: @Mixin target com.chocohead.test.Missing was not found tests.mixins.json:MissingTargetMixin from mod test-mod
[WARN] [FabricLoader/Mixin]: Error loading class: com/chocohead/test/Missing (java.lang.ClassNotFoundException: com/chocohead/test/Missing)
[DEBUG] [FabricLoader/Mixin]: Inner class com/chocohead/test/mixin/MissingTargetMixin$Inner in com/chocohead/test/mixin/MissingTargetMixin on com/chocohead/test/target/Target gets unique name com/chocohead/test/target/Target$Inner$95f9752e1a2243388dc678dfab05f815
[WARN] [FabricLoader/Mixin]: Error loading class: com/chocohead/test/Missing (java.lang.ClassNotFoundException: com/chocohead/test/Missing)
[ERROR] [FabricLoader/Mixin]: Prepare error for tests.mixins.json:MissingTargetMixin from mod test-mod during activity: [Prepare inner classes]
org.spongepowered.asm.mixin.transformer.throwables.MixinPreProcessorException: Prepare error for tests.mixins.json:MissingTargetMixin from mod test-mod during activity: [Prepare inner classes]
	at org.spongepowered.asm.mixin.transformer.MixinPreProcessorStandard.prepare(MixinPreProcessorStandard.java:197)
	at org.spongepowered.asm.mixin.transformer.MixinInfo$State.validateChanges(MixinInfo.java:442)
	at org.spongepowered.asm.mixin.transformer.MixinInfo$State.validate(MixinInfo.java:342)
	at org.spongepowered.asm.mixin.transformer.MixinInfo.validate(MixinInfo.java:913)
	at org.spongepowered.asm.mixin.transformer.MixinConfig.postInitialise(MixinConfig.java:884)
	at org.spongepowered.asm.mixin.transformer.MixinProcessor.prepareConfigs(MixinProcessor.java:568)
	at org.spongepowered.asm.mixin.transformer.MixinProcessor.select(MixinProcessor.java:462)
	at org.spongepowered.asm.mixin.transformer.MixinProcessor.checkSelect(MixinProcessor.java:438)
	at org.spongepowered.asm.mixin.transformer.MixinProcessor.applyMixins(MixinProcessor.java:290)
	at org.spongepowered.asm.mixin.transformer.MixinTransformer.transformClass(MixinTransformer.java:234)
	at org.spongepowered.asm.mixin.transformer.MixinTransformer.transformClassBytes(MixinTransformer.java:202)
	at net.fabricmc.loader.impl.launch.knot.KnotClassDelegate.getPostMixinClassByteArray(KnotClassDelegate.java:422)
	at net.fabricmc.loader.impl.launch.knot.KnotClassDelegate.tryLoadClass(KnotClassDelegate.java:323)
	at net.fabricmc.loader.impl.launch.knot.KnotClassDelegate.loadClass(KnotClassDelegate.java:218)
	at net.fabricmc.loader.impl.launch.knot.KnotClassLoader.loadClass(KnotClassLoader.java:119)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	at java.lang.Class.forName0(Native Method)
	at java.lang.Class.forName(Class.java:348)
	at net.fabricmc.loader.impl.util.DefaultLanguageAdapter.create(DefaultLanguageAdapter.java:50)
	at net.fabricmc.loader.impl.entrypoint.EntrypointStorage$NewEntry.getOrCreate(EntrypointStorage.java:124)
	at net.fabricmc.loader.impl.entrypoint.EntrypointContainerImpl.getEntrypoint(EntrypointContainerImpl.java:53)
	at net.fabricmc.loader.impl.FabricLoaderImpl.invokeEntrypoints(FabricLoaderImpl.java:399)
	at com.chocohead.wim.Main.main(Main.java:20)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at com.chocohead.wim.Provider.launch(Provider.java:129)
	at net.fabricmc.loader.impl.launch.knot.Knot.launch(Knot.java:74)
	at net.fabricmc.loader.impl.launch.knot.KnotClient.main(KnotClient.java:23)
Caused by: java.lang.NullPointerException
	at org.spongepowered.asm.mixin.transformer.InnerClassGenerator.registerInnerClass(InnerClassGenerator.java:341)
	at org.spongepowered.asm.mixin.transformer.MixinPreProcessorStandard.prepareInnerClasses(MixinPreProcessorStandard.java:209)
	at org.spongepowered.asm.mixin.transformer.MixinPreProcessorStandard.prepare(MixinPreProcessorStandard.java:178)
	... 29 more

This PR corrects this by avoiding resolving each target's ClassInfo every time a mixin is loaded, making the most of the targets having already been resolved when the mixin config was selected. It also avoids iterating the targets list at all when a mixin has no inner classes, given most mixins will not.

[LlamaLad7]

This seems to be 2 changes: fixing the bug, and avoiding empty list iterations. I agree with the first but not with the second, seems like a divergence for the sake of micro-optimization. Can we just fix the bug?

[Chocohead]

The optimisation scales quadratically with the number of targets a mixin has:

• If all you have are mixins with one target, the difference is negligible as each mixin loops over an empty collection once.
• If you have a mixin with 43000 targets, suddenly you've done 1.85 billion loops over an empty collection once all 43000 classes are loaded.

Loading large modpacks (such as Blanketcon-25) with Not So New will produce a mixin with target counts in this kind of range. At this scale the optimisation is not micro anymore.

Whilst I will happily admit it's hardly a common use, this PR pushes the problem down to people with mixins with huge target counts and inner classes, which is an even less common use, at no expense to the standard uses. It also should make profiling a little easier to find what is still so slow in my use.

[LlamaLad7]

Why not swap the loops around so the inner class one is on the outside?

[LlamaLad7]

Though as a general point I'm quite against diverging from upstream to optimise things purely for the sake of Not So New, which as far as I can ascertain has no practical use other than curiosity.

[Chocohead]

Ironically I'd left the loops that way around to minimise the changes to what was already there.

The divergence from upstream is when the bug is fixed, from that point changing the lines immediately around it is inconsequential given it all forms a single patch together anyway. Nothing is stopping this change from going into upstream either; beyond the glacial pace anything is pushed there to begin with.

Although were upstream in the mood for fixing this, it should really be done by InnerClassGenerator#registerInnerClass only being called once (per inner class) per mixin, rather than repeating as each target is applied. I didn't think the extra differences to achieve that were worth us doing it however; I felt these changes were the best middle ground.

[LlamaLad7]

from that point changing the lines immediately around it is inconsequential

I agree, but your suggestion also adds an extra method to MixinInfo and mine doesn't.

[Chocohead]

Adding is much less consequential, it just has to fit somewhere around what is already there, as opposed to patched on top of what is there. It would get much more micro-optimisation-y if a UnmodifiableSet was created just to check if it was empty before looping over a new UnmodifiableSet of the same contents.