Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid serializing object-based payload into PacketByteBuf on the main thread #3407

Merged
merged 7 commits into from
Nov 26, 2023
Merged

Avoid serializing object-based payload into PacketByteBuf on the main thread #3407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bf76eba
channeled network addon refactor
deirn Nov 6, 2023
a112668
checkstyle
deirn Nov 7, 2023
56eecf7
fix junit tests
deirn Nov 7, 2023
7c3648e
convert TypedPayload <-> UntypedPayload if necessary
deirn Nov 7, 2023
3573e5a
assert payload size
deirn Nov 20, 2023
48b5852
add vm arg to force serialization
deirn Nov 20, 2023
49505ea
change log level to info and make it single line
deirn Nov 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 64 additions & 64 deletions ...ient/java/net/fabricmc/fabric/api/client/networking/v1/ClientConfigurationNetworking.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@
import net.fabricmc.fabric.api.networking.v1.FabricPacket;
import net.fabricmc.fabric.api.networking.v1.PacketSender;
import net.fabricmc.fabric.api.networking.v1.PacketType;
import net.fabricmc.fabric.api.networking.v1.ServerConfigurationNetworking;
import net.fabricmc.fabric.impl.networking.client.ClientConfigurationNetworkAddon;
import net.fabricmc.fabric.impl.networking.client.ClientNetworkingImpl;
import net.fabricmc.fabric.impl.networking.payload.ResolvablePayload;
import net.fabricmc.fabric.impl.networking.payload.TypedPayload;
import net.fabricmc.fabric.impl.networking.payload.UntypedPayload;
import net.fabricmc.fabric.mixin.networking.client.accessor.ClientCommonNetworkHandlerAccessor;

/**
Expand Down Expand Up @@ -71,7 +73,7 @@ public final class ClientConfigurationNetworking {
* @see ClientConfigurationNetworking#registerReceiver(Identifier, ConfigurationChannelHandler)
*/
public static boolean registerGlobalReceiver(Identifier channelName, ConfigurationChannelHandler channelHandler) {
return ClientNetworkingImpl.CONFIGURATION.registerGlobalReceiver(channelName, channelHandler);
return ClientNetworkingImpl.CONFIGURATION.registerGlobalReceiver(channelName, wrapUntyped(channelHandler));
}

/**
Expand All @@ -88,29 +90,7 @@ public static boolean registerGlobalReceiver(Identifier channelName, Configurati
* @see ClientConfigurationNetworking#registerReceiver(PacketType, ConfigurationPacketHandler)
*/
public static <T extends FabricPacket> boolean registerGlobalReceiver(PacketType<T> type, ConfigurationPacketHandler<T> handler) {
return registerGlobalReceiver(type.getId(), new ConfigurationChannelHandlerProxy<T>() {
@Override
public ConfigurationPacketHandler<T> getOriginalHandler() {
return handler;
}

@Override
public void receive(MinecraftClient client, ClientConfigurationNetworkHandler networkHandler, PacketByteBuf buf, PacketSender sender) {
T packet = type.read(buf);

if (client.isOnThread()) {
// Do not submit to the render thread if we're already running there.
// Normally, packets are handled on the network IO thread - though it is
// not guaranteed (for example, with 1.19.4 S2C packet bundling)
// Since we're handling it right now, connection check is redundant.
handler.receive(packet, sender);
} else {
client.execute(() -> {
if (((ClientCommonNetworkHandlerAccessor) networkHandler).getConnection().isOpen()) handler.receive(packet, sender);
});
}
}
});
return ClientNetworkingImpl.CONFIGURATION.registerGlobalReceiver(type.getId(), wrapTyped(type, handler));
}

/**
Expand All @@ -126,7 +106,7 @@ public void receive(MinecraftClient client, ClientConfigurationNetworkHandler ne
*/
@Nullable
public static ClientConfigurationNetworking.ConfigurationChannelHandler unregisterGlobalReceiver(Identifier channelName) {
return ClientNetworkingImpl.CONFIGURATION.unregisterGlobalReceiver(channelName);
return unwrapUntyped(ClientNetworkingImpl.CONFIGURATION.unregisterGlobalReceiver(channelName));
}

/**
Expand All @@ -142,10 +122,8 @@ public static ClientConfigurationNetworking.ConfigurationChannelHandler unregist
* @see ClientConfigurationNetworking#unregisterReceiver(PacketType)
*/
@Nullable
@SuppressWarnings("unchecked")
public static <T extends FabricPacket> ClientConfigurationNetworking.ConfigurationPacketHandler<T> unregisterGlobalReceiver(PacketType<T> type) {
ConfigurationChannelHandler handler = ClientNetworkingImpl.CONFIGURATION.unregisterGlobalReceiver(type.getId());
return handler instanceof ConfigurationChannelHandlerProxy<?> proxy ? (ConfigurationPacketHandler<T>) proxy.getOriginalHandler() : null;
return unwrapTyped(ClientNetworkingImpl.CONFIGURATION.unregisterGlobalReceiver(type.getId()));
}

/**
Expand Down Expand Up @@ -179,7 +157,7 @@ public static boolean registerReceiver(Identifier channelName, ConfigurationChan
final ClientConfigurationNetworkAddon addon = ClientNetworkingImpl.getClientConfigurationAddon();

if (addon != null) {
return addon.registerChannel(channelName, channelHandler);
return addon.registerChannel(channelName, wrapUntyped(channelHandler));
}

throw new IllegalStateException("Cannot register receiver while not configuring!");
Expand All @@ -201,29 +179,13 @@ public static boolean registerReceiver(Identifier channelName, ConfigurationChan
* @see ClientPlayConnectionEvents#INIT
*/
public static <T extends FabricPacket> boolean registerReceiver(PacketType<T> type, ConfigurationPacketHandler<T> handler) {
return registerReceiver(type.getId(), new ConfigurationChannelHandlerProxy<T>() {
@Override
public ConfigurationPacketHandler<T> getOriginalHandler() {
return handler;
}
final ClientConfigurationNetworkAddon addon = ClientNetworkingImpl.getClientConfigurationAddon();

@Override
public void receive(MinecraftClient client, ClientConfigurationNetworkHandler networkHandler, PacketByteBuf buf, PacketSender sender) {
T packet = type.read(buf);

if (client.isOnThread()) {
// Do not submit to the render thread if we're already running there.
// Normally, packets are handled on the network IO thread - though it is
// not guaranteed (for example, with 1.19.4 S2C packet bundling)
// Since we're handling it right now, connection check is redundant.
handler.receive(packet, sender);
} else {
client.execute(() -> {
if (((ClientCommonNetworkHandlerAccessor) networkHandler).getConnection().isOpen()) handler.receive(packet, sender);
});
}
}
});
if (addon != null) {
return addon.registerChannel(type.getId(), wrapTyped(type, handler));
}

throw new IllegalStateException("Cannot register receiver while not configuring!");
}

/**
Expand All @@ -240,7 +202,7 @@ public static ClientConfigurationNetworking.ConfigurationChannelHandler unregist
final ClientConfigurationNetworkAddon addon = ClientNetworkingImpl.getClientConfigurationAddon();

if (addon != null) {
return addon.unregisterChannel(channelName);
return unwrapUntyped(addon.unregisterChannel(channelName));
}

throw new IllegalStateException("Cannot unregister receiver while not configuring!");
Expand All @@ -257,10 +219,14 @@ public static ClientConfigurationNetworking.ConfigurationChannelHandler unregist
* @throws IllegalStateException if the client is not connected to a server
*/
@Nullable
@SuppressWarnings("unchecked")
public static <T extends FabricPacket> ClientConfigurationNetworking.ConfigurationPacketHandler<T> unregisterReceiver(PacketType<T> type) {
ConfigurationChannelHandler handler = unregisterReceiver(type.getId());
return handler instanceof ConfigurationChannelHandlerProxy<?> proxy ? (ConfigurationPacketHandler<T>) proxy.getOriginalHandler() : null;
final ClientConfigurationNetworkAddon addon = ClientNetworkingImpl.getClientConfigurationAddon();

if (addon != null) {
return unwrapTyped(addon.unregisterChannel(type.getId()));
}

throw new IllegalStateException("Cannot unregister receiver while not configuring!");
}

/**
Expand Down Expand Up @@ -394,6 +360,48 @@ public static <T extends FabricPacket> void send(T packet) {
private ClientConfigurationNetworking() {
}

private static ResolvablePayload.Handler<ClientConfigurationNetworkAddon.Handler> wrapUntyped(ConfigurationChannelHandler actualHandler) {
return new ResolvablePayload.Handler<>(null, actualHandler, (client, handler, payload, responseSender) -> {
actualHandler.receive(client, handler, ((UntypedPayload) payload).buffer(), responseSender);
});
}

@SuppressWarnings("unchecked")
private static <T extends FabricPacket> ResolvablePayload.Handler<ClientConfigurationNetworkAddon.Handler> wrapTyped(PacketType<T> type, ConfigurationPacketHandler<T> actualHandler) {
return new ResolvablePayload.Handler<>(type, actualHandler, (client, handler, payload, responseSender) -> {
T packet = (T) ((TypedPayload) payload).packet();

if (client.isOnThread()) {
// Do not submit to the render thread if we're already running there.
// Normally, packets are handled on the network IO thread - though it is
// not guaranteed (for example, with 1.19.4 S2C packet bundling)
// Since we're handling it right now, connection check is redundant.
actualHandler.receive(packet, responseSender);
} else {
client.execute(() -> {
if (((ClientCommonNetworkHandlerAccessor) handler).getConnection().isOpen()) {
actualHandler.receive(packet, responseSender);
}
});
}
});
}

@Nullable
private static ConfigurationChannelHandler unwrapUntyped(@Nullable ResolvablePayload.Handler<ClientConfigurationNetworkAddon.Handler> handler) {
if (handler == null) return null;
if (handler.actual() instanceof ConfigurationChannelHandler actual) return actual;
return null;
}

@Nullable
@SuppressWarnings({"rawtypes", "unchecked"})
private static <T extends FabricPacket> ConfigurationPacketHandler<T> unwrapTyped(@Nullable ResolvablePayload.Handler<ClientConfigurationNetworkAddon.Handler> handler) {
if (handler == null) return null;
if (handler.actual() instanceof ConfigurationPacketHandler actual) return actual;
return null;
}

@FunctionalInterface
public interface ConfigurationChannelHandler {
/**
Expand Down Expand Up @@ -421,14 +429,6 @@ public interface ConfigurationChannelHandler {
void receive(MinecraftClient client, ClientConfigurationNetworkHandler handler, PacketByteBuf buf, PacketSender responseSender);
}

/**
* An internal packet handler that works as a proxy between old and new API.
* @param <T> the type of the packet
*/
private interface ConfigurationChannelHandlerProxy<T extends FabricPacket> extends ConfigurationChannelHandler {
ConfigurationPacketHandler<T> getOriginalHandler();
}

/**
* A thread-safe packet handler utilizing {@link FabricPacket}.
* @param <T> the type of the packet
Expand Down
Loading
Loading