Stop caching constructor parameters

Addresses #2990
This commit is contained in:
Dan Mulloy 2024-06-08 14:09:23 -05:00
parent b6a7d5f91d
commit d13ce24507
No known key found for this signature in database
GPG Key ID: 3C5AD5D866D1539A
4 changed files with 88 additions and 79 deletions

View File

@ -31,6 +31,7 @@ import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.function.Function;
import javax.annotation.Nullable;
import com.comphenix.protocol.PacketType;
import com.comphenix.protocol.injector.StructureCache;
@ -58,10 +59,10 @@ import com.comphenix.protocol.utility.MinecraftReflection;
import com.comphenix.protocol.utility.MinecraftVersion;
import com.comphenix.protocol.wrappers.Converters;
import com.comphenix.protocol.wrappers.WrappedStreamCodec;
import com.google.common.collect.Sets;
import io.netty.buffer.ByteBuf;
import io.netty.util.ReferenceCountUtil;
import javax.annotation.Nullable;
/**
* Represents a Minecraft packet indirectly.
@ -80,7 +81,7 @@ public class PacketContainer extends AbstractStructure implements Serializable {
// Used to clone packets
private static final AggregateCloner DEEP_CLONER = AggregateCloner
.newBuilder()
.instanceProvider(StructureCache::newPacket)
.instanceProvider(StructureCache::newInstance)
.andThen(BukkitCloner.class)
.andThen(ImmutableDetector.class)
.andThen(JavaOptionalCloner.class)
@ -91,7 +92,7 @@ public class PacketContainer extends AbstractStructure implements Serializable {
private static final AggregateCloner SHALLOW_CLONER = AggregateCloner
.newBuilder()
.instanceProvider(StructureCache::newPacket)
.instanceProvider(StructureCache::newInstance)
.andThen(param -> {
if (param == null)
throw new IllegalArgumentException("Cannot be NULL.");

View File

@ -32,7 +32,7 @@ import com.comphenix.protocol.reflect.accessors.Accessors;
import com.comphenix.protocol.reflect.accessors.ConstructorAccessor;
import com.comphenix.protocol.reflect.fuzzy.FuzzyMethodContract;
import com.comphenix.protocol.reflect.instances.DefaultInstances;
import com.comphenix.protocol.reflect.instances.PacketCreator;
import com.comphenix.protocol.reflect.instances.InstanceCreator;
import com.comphenix.protocol.utility.ByteBuddyFactory;
import com.comphenix.protocol.utility.MinecraftMethods;
import com.comphenix.protocol.utility.MinecraftReflection;
@ -57,7 +57,7 @@ import net.bytebuddy.matcher.ElementMatchers;
public class StructureCache {
// Structure modifiers
private static final Map<Class<?>, Supplier<Object>> PACKET_INSTANCE_CREATORS = new ConcurrentHashMap<>();
private static final Map<Class<?>, Supplier<Object>> CACHED_INSTANCE_CREATORS = new ConcurrentHashMap<>();
private static final Map<PacketType, StructureModifier<Object>> STRUCTURE_MODIFIER_CACHE = new ConcurrentHashMap<>();
// packet data serializer which always returns an empty nbt tag compound
@ -67,80 +67,90 @@ public class StructureCache {
private static Supplier<Object> TRICKED_DATA_SERIALIZER_BASE;
private static Supplier<Object> TRICKED_DATA_SERIALIZER_JSON;
/**
* @deprecated Renamed to {@link #newInstance(Class)}.
*/
@Deprecated
public static Object newPacket(Class<?> packetClass) {
Supplier<Object> packetConstructor = PACKET_INSTANCE_CREATORS.computeIfAbsent(packetClass, packetClassKey -> {
try {
PacketCreator creator = PacketCreator.forPacket(packetClassKey);
if (creator.get() != null) {
return creator;
}
} catch (Exception ignored) {
return newInstance(packetClass);
}
public static Object newInstance(Class<?> clazz) {
Supplier<Object> creator = CACHED_INSTANCE_CREATORS.computeIfAbsent(clazz, StructureCache::determineBestCreator);
return creator.get();
}
static Supplier<Object> determineBestCreator(Class<?> clazz) {
try {
InstanceCreator creator = InstanceCreator.forClass(clazz);
if (creator.get() != null) {
return creator;
}
} catch (Exception ignored) {
}
WrappedStreamCodec streamCodec = PacketRegistry.getStreamCodec(packetClassKey);
WrappedStreamCodec streamCodec = PacketRegistry.getStreamCodec(clazz);
// use the new stream codec for versions above 1.20.5 if possible
if (streamCodec != null && tryInitTrickDataSerializer()) {
// use the new stream codec for versions above 1.20.5 if possible
if (streamCodec != null && tryInitTrickDataSerializer()) {
try {
// first try with the base accessor
Object serializer = TRICKED_DATA_SERIALIZER_BASE.get();
streamCodec.decode(serializer); // throwaway instance, for testing
// method is working
return () -> streamCodec.decode(serializer);
} catch (Exception ignored) {
try {
// first try with the base accessor
Object serializer = TRICKED_DATA_SERIALIZER_BASE.get();
// try with the json accessor
Object serializer = TRICKED_DATA_SERIALIZER_JSON.get();
streamCodec.decode(serializer); // throwaway instance, for testing
// method is working
return () -> streamCodec.decode(serializer);
} catch (Exception ignored) {
try {
// try with the json accessor
Object serializer = TRICKED_DATA_SERIALIZER_JSON.get();
streamCodec.decode(serializer); // throwaway instance, for testing
// method is working
return () -> streamCodec.decode(serializer);
} catch (Exception ignored1) {
// shrug, fall back to default behaviour
}
} catch (Exception ignored1) {
// shrug, fall back to default behaviour
}
}
}
// prefer construction via PacketDataSerializer constructor on 1.17 and above
if (MinecraftVersion.CAVES_CLIFFS_1.atOrAbove()) {
ConstructorAccessor serializerAccessor = Accessors.getConstructorAccessorOrNull(
packetClassKey,
MinecraftReflection.getPacketDataSerializerClass());
if (serializerAccessor != null) {
// check if the method is possible
if (tryInitTrickDataSerializer()) {
// prefer construction via PacketDataSerializer constructor on 1.17 and above
if (MinecraftVersion.CAVES_CLIFFS_1.atOrAbove()) {
ConstructorAccessor serializerAccessor = Accessors.getConstructorAccessorOrNull(
clazz,
MinecraftReflection.getPacketDataSerializerClass());
if (serializerAccessor != null) {
// check if the method is possible
if (tryInitTrickDataSerializer()) {
try {
// first try with the base accessor
Object serializer = TRICKED_DATA_SERIALIZER_BASE.get();
serializerAccessor.invoke(serializer); // throwaway instance, for testing
// method is working
return () -> serializerAccessor.invoke(serializer);
} catch (Exception ignored) {
try {
// first try with the base accessor
Object serializer = TRICKED_DATA_SERIALIZER_BASE.get();
// try with the json accessor
Object serializer = TRICKED_DATA_SERIALIZER_JSON.get();
serializerAccessor.invoke(serializer); // throwaway instance, for testing
// method is working
return () -> serializerAccessor.invoke(serializer);
} catch (Exception ignored) {
try {
// try with the json accessor
Object serializer = TRICKED_DATA_SERIALIZER_JSON.get();
serializerAccessor.invoke(serializer); // throwaway instance, for testing
// method is working
return () -> serializerAccessor.invoke(serializer);
} catch (Exception ignored1) {
// shrug, fall back to default behaviour
}
} catch (Exception ignored1) {
// shrug, fall back to default behaviour
}
}
}
}
}
// try via DefaultInstances as fallback
return () -> {
Object packetInstance = DefaultInstances.DEFAULT.create(packetClassKey);
Objects.requireNonNull(packetInstance, "Unable to create packet instance for class " + packetClassKey + " - " + tryInitTrickDataSerializer() + " - " + streamCodec);
return packetInstance;
};
});
return packetConstructor.get();
// try via DefaultInstances as fallback
return () -> {
Object packetInstance = DefaultInstances.DEFAULT.create(clazz);
Objects.requireNonNull(packetInstance, "Unable to create instance for class " + clazz + " - " + tryInitTrickDataSerializer() + " - " + streamCodec);
return packetInstance;
};
}
/**

View File

@ -8,30 +8,25 @@ import java.util.function.Supplier;
import com.comphenix.protocol.reflect.accessors.Accessors;
import com.comphenix.protocol.reflect.accessors.ConstructorAccessor;
import com.comphenix.protocol.reflect.accessors.MethodAccessor;
import com.comphenix.protocol.utility.MinecraftReflection;
public final class PacketCreator implements Supplier<Object> {
public final class InstanceCreator implements Supplier<Object> {
private ConstructorAccessor constructor = null;
private MethodAccessor factoryMethod = null;
private Object[] params = null;
private Class<?>[] paramTypes = null;
private boolean failed = false;
private final Class<?> type;
private PacketCreator(Class<?> type) {
private InstanceCreator(Class<?> type) {
this.type = type;
}
public static PacketCreator forPacket(Class<?> type) {
public static InstanceCreator forClass(Class<?> type) {
if (type == null) {
throw new IllegalArgumentException("Type cannot be null.");
}
if (!MinecraftReflection.getPacketClass().isAssignableFrom(type)) {
throw new IllegalArgumentException("Type must be a subclass of Packet.");
}
return new PacketCreator(type);
return new InstanceCreator(type);
}
private Object createInstance(Class<?> clazz) {
@ -42,8 +37,18 @@ public final class PacketCreator implements Supplier<Object> {
}
}
private Object[] createParams(Class<?>[] paramTypes) {
Object[] params = new Object[paramTypes.length];
for (int i = 0; i < paramTypes.length; i++) {
params[i] = createInstance(paramTypes[i]);
}
return params;
}
@Override
public Object get() {
Object[] params = paramTypes != null ? createParams(paramTypes) : null;
if (constructor != null) {
return constructor.invoke(params);
}
@ -65,16 +70,13 @@ public final class PacketCreator implements Supplier<Object> {
continue;
}
Object[] testParams = new Object[paramTypes.length];
for (int i = 0; i < paramTypes.length; i++) {
testParams[i] = createInstance(paramTypes[i]);
}
Object[] testParams = createParams(paramTypes);
try {
result = testCtor.newInstance(testParams);
minCount = paramTypes.length;
this.constructor = Accessors.getConstructorAccessor(testCtor);
this.params = testParams;
this.paramTypes = paramTypes;
} catch (Exception ignored) {
}
}
@ -100,16 +102,13 @@ public final class PacketCreator implements Supplier<Object> {
continue;
}
Object[] testParams = new Object[paramTypes.length];
for (int i = 0; i < paramTypes.length; i++) {
testParams[i] = createInstance(paramTypes[i]);
}
Object[] testParams = createParams(paramTypes);
try {
result = testMethod.invoke(null, testParams);
minCount = paramTypes.length;
this.factoryMethod = Accessors.getMethodAccessor(testMethod);
this.params = testParams;
this.paramTypes = paramTypes;
} catch (Exception ignored) {
}
}

View File

@ -154,8 +154,7 @@ public class WrappedTeamParameters extends AbstractWrapper {
Preconditions.checkNotNull(collisionRule, "Collision rule not set");
Preconditions.checkNotNull(color, "Color not set");
// Not technically a packet, but it has a PacketDataSerializer constructor, so it works fine
Object handle = StructureCache.newPacket(getNmsClassOrThrow());
Object handle = StructureCache.newInstance(getNmsClassOrThrow());
WrappedTeamParameters wrapped = new WrappedTeamParameters(handle);
wrapped.writeComponent(0, displayName);