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.

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

Improve MCPL ticking data and behaviour #865

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@
public interface Packet {

/**
* Gets whether the packet has handling priority.
* If the result is true, the packet will be handled immediately after being
* decoded.
* Gets whether the packet should run on an async game thread rather than blocking the network (Netty) thread.
* Packets that qualify for this are usually packets with an ensureRunningOnSameThread call at the top of their packet listener method in the Minecraft code.
* Packets which need extra attention because they aren't "fully" handled async are marked using.
* // GAME THREAD DETAIL comments in the MCProtocolLib code.
*
* @return Whether the packet has priority.
* @return Whether the packet be handled async from the Netty thread.
*/
default boolean isPriority() {
default boolean shouldRunOnGameThread() {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package org.geysermc.mcprotocollib.network.tcp;

import io.netty.channel.DefaultEventLoopGroup;
import io.netty.channel.EventLoopGroup;
import io.netty.util.concurrent.DefaultThreadFactory;

import java.util.concurrent.Executor;
import java.util.concurrent.TimeUnit;

public class DefaultPacketHandlerExecutor {
/**
* Controls whether non-priority packets are handled in a separate event loop
*/
public static boolean USE_EVENT_LOOP_FOR_PACKETS = true;
private static EventLoopGroup PACKET_EVENT_LOOP;
private static final int SHUTDOWN_QUIET_PERIOD_MS = 100;
private static final int SHUTDOWN_TIMEOUT_MS = 500;

public static Executor createExecutor() {
if (!USE_EVENT_LOOP_FOR_PACKETS) {
return Runnable::run;
}

if (PACKET_EVENT_LOOP == null) {
// See TcpClientSession.newThreadFactory() for details on
// daemon threads and their interaction with the runtime.
PACKET_EVENT_LOOP = new DefaultEventLoopGroup(new DefaultThreadFactory(DefaultPacketHandlerExecutor.class, true));
Runtime.getRuntime().addShutdownHook(new Thread(
() -> PACKET_EVENT_LOOP.shutdownGracefully(SHUTDOWN_QUIET_PERIOD_MS, SHUTDOWN_TIMEOUT_MS, TimeUnit.MILLISECONDS)));
}

return PACKET_EVENT_LOOP.next();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import java.net.InetSocketAddress;
import java.net.UnknownHostException;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.Executor;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.TimeUnit;

Expand Down Expand Up @@ -76,7 +77,11 @@ public TcpClientSession(String host, int port, String bindAddress, int bindPort,
}

public TcpClientSession(String host, int port, String bindAddress, int bindPort, PacketProtocol protocol, ProxyInfo proxy) {
super(host, port, protocol);
this(host, port, bindAddress, bindPort, protocol, proxy, DefaultPacketHandlerExecutor.createExecutor());
}

public TcpClientSession(String host, int port, String bindAddress, int bindPort, PacketProtocol protocol, ProxyInfo proxy, Executor packetHandlerExecutor) {
super(host, port, protocol, packetHandlerExecutor);
this.bindAddress = bindAddress;
this.bindPort = bindPort;
this.proxy = proxy;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public class TcpServerSession extends TcpSession {
private final PacketCodecHelper codecHelper;

public TcpServerSession(String host, int port, PacketProtocol protocol, TcpServer server) {
super(host, port, protocol);
super(host, port, protocol, DefaultPacketHandlerExecutor.createExecutor());
this.server = server;
this.codecHelper = protocol.createHelper();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,7 @@
import io.netty.channel.Channel;
import io.netty.channel.ChannelFutureListener;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.DefaultEventLoopGroup;
import io.netty.channel.EventLoop;
import io.netty.channel.EventLoopGroup;
import io.netty.channel.SimpleChannelInboundHandler;
import io.netty.util.concurrent.DefaultThreadFactory;
import net.kyori.adventure.text.Component;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;
Expand All @@ -33,35 +29,28 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.Executor;
import java.util.concurrent.TimeoutException;

public abstract class TcpSession extends SimpleChannelInboundHandler<Packet> implements Session {
private static final Logger log = LoggerFactory.getLogger(TcpSession.class);

/**
* Controls whether non-priority packets are handled in a separate event loop
*/
public static boolean USE_EVENT_LOOP_FOR_PACKETS = true;
private static EventLoopGroup PACKET_EVENT_LOOP;
private static final int SHUTDOWN_QUIET_PERIOD_MS = 100;
private static final int SHUTDOWN_TIMEOUT_MS = 500;

protected String host;
protected int port;
private final PacketProtocol protocol;
private final EventLoop eventLoop = createEventLoop();
private final Executor packetHandlerExecutor;

private final Map<String, Object> flags = new HashMap<>();
private final List<SessionListener> listeners = new CopyOnWriteArrayList<>();

private Channel channel;
protected boolean disconnected = false;

public TcpSession(String host, int port, PacketProtocol protocol) {
public TcpSession(String host, int port, PacketProtocol protocol, Executor packetHandlerExecutor) {
this.host = host;
this.port = port;
this.protocol = protocol;
this.packetHandlerExecutor = packetHandlerExecutor;
}

@Override
Expand Down Expand Up @@ -269,21 +258,6 @@ public void setAutoRead(boolean autoRead) {
}
}

private @Nullable EventLoop createEventLoop() {
if (!USE_EVENT_LOOP_FOR_PACKETS) {
return null;
}

if (PACKET_EVENT_LOOP == null) {
// See TcpClientSession.newThreadFactory() for details on
// daemon threads and their interaction with the runtime.
PACKET_EVENT_LOOP = new DefaultEventLoopGroup(new DefaultThreadFactory(this.getClass(), true));
Runtime.getRuntime().addShutdownHook(new Thread(
() -> PACKET_EVENT_LOOP.shutdownGracefully(SHUTDOWN_QUIET_PERIOD_MS, SHUTDOWN_TIMEOUT_MS, TimeUnit.MILLISECONDS)));
}
return PACKET_EVENT_LOOP.next();
}

@Override
public Channel getChannel() {
return this.channel;
Expand Down Expand Up @@ -322,8 +296,8 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {

@Override
protected void channelRead0(ChannelHandlerContext ctx, Packet packet) {
if (!packet.isPriority() && eventLoop != null) {
eventLoop.execute(() -> this.callPacketReceived(packet));
if (packet.shouldRunOnGameThread()) {
packetHandlerExecutor.execute(() -> this.callPacketReceived(packet));
} else {
this.callPacketReceived(packet);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@
import org.geysermc.mcprotocollib.protocol.packet.login.serverbound.ServerboundHelloPacket;
import org.geysermc.mcprotocollib.protocol.packet.login.serverbound.ServerboundKeyPacket;
import org.geysermc.mcprotocollib.protocol.packet.login.serverbound.ServerboundLoginAcknowledgedPacket;
import org.geysermc.mcprotocollib.protocol.packet.status.clientbound.ClientboundPongResponsePacket;
import org.geysermc.mcprotocollib.protocol.packet.ping.clientbound.ClientboundPongResponsePacket;
import org.geysermc.mcprotocollib.protocol.packet.status.clientbound.ClientboundStatusResponsePacket;
import org.geysermc.mcprotocollib.protocol.packet.status.serverbound.ServerboundPingRequestPacket;
import org.geysermc.mcprotocollib.protocol.packet.ping.serverbound.ServerboundPingRequestPacket;
import org.geysermc.mcprotocollib.protocol.packet.status.serverbound.ServerboundStatusRequestPacket;

import javax.crypto.KeyGenerator;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@
import org.geysermc.mcprotocollib.protocol.packet.login.serverbound.ServerboundHelloPacket;
import org.geysermc.mcprotocollib.protocol.packet.login.serverbound.ServerboundKeyPacket;
import org.geysermc.mcprotocollib.protocol.packet.login.serverbound.ServerboundLoginAcknowledgedPacket;
import org.geysermc.mcprotocollib.protocol.packet.status.clientbound.ClientboundPongResponsePacket;
import org.geysermc.mcprotocollib.protocol.packet.ping.clientbound.ClientboundPongResponsePacket;
import org.geysermc.mcprotocollib.protocol.packet.status.clientbound.ClientboundStatusResponsePacket;
import org.geysermc.mcprotocollib.protocol.packet.status.serverbound.ServerboundPingRequestPacket;
import org.geysermc.mcprotocollib.protocol.packet.ping.serverbound.ServerboundPingRequestPacket;
import org.geysermc.mcprotocollib.protocol.packet.status.serverbound.ServerboundStatusRequestPacket;

import javax.crypto.SecretKey;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package org.geysermc.mcprotocollib.protocol.codec;

import org.geysermc.mcprotocollib.protocol.data.ProtocolState;
import org.geysermc.mcprotocollib.protocol.packet.common.clientbound.ClientboundCookieRequestPacket;
import org.geysermc.mcprotocollib.protocol.packet.cookie.clientbound.ClientboundCookieRequestPacket;
import org.geysermc.mcprotocollib.protocol.packet.common.clientbound.ClientboundCustomPayloadPacket;
import org.geysermc.mcprotocollib.protocol.packet.common.clientbound.ClientboundCustomReportDetailsPacket;
import org.geysermc.mcprotocollib.protocol.packet.common.clientbound.ClientboundDisconnectPacket;
Expand All @@ -13,7 +13,7 @@
import org.geysermc.mcprotocollib.protocol.packet.common.clientbound.ClientboundStoreCookiePacket;
import org.geysermc.mcprotocollib.protocol.packet.common.clientbound.ClientboundTransferPacket;
import org.geysermc.mcprotocollib.protocol.packet.common.clientbound.ClientboundUpdateTagsPacket;
import org.geysermc.mcprotocollib.protocol.packet.common.clientbound.ServerboundCookieResponsePacket;
import org.geysermc.mcprotocollib.protocol.packet.cookie.serverbound.ServerboundCookieResponsePacket;
import org.geysermc.mcprotocollib.protocol.packet.common.serverbound.ServerboundClientInformationPacket;
import org.geysermc.mcprotocollib.protocol.packet.common.serverbound.ServerboundCustomPayloadPacket;
import org.geysermc.mcprotocollib.protocol.packet.common.serverbound.ServerboundKeepAlivePacket;
Expand Down Expand Up @@ -207,9 +207,9 @@
import org.geysermc.mcprotocollib.protocol.packet.login.serverbound.ServerboundHelloPacket;
import org.geysermc.mcprotocollib.protocol.packet.login.serverbound.ServerboundKeyPacket;
import org.geysermc.mcprotocollib.protocol.packet.login.serverbound.ServerboundLoginAcknowledgedPacket;
import org.geysermc.mcprotocollib.protocol.packet.status.clientbound.ClientboundPongResponsePacket;
import org.geysermc.mcprotocollib.protocol.packet.ping.clientbound.ClientboundPongResponsePacket;
import org.geysermc.mcprotocollib.protocol.packet.status.clientbound.ClientboundStatusResponsePacket;
import org.geysermc.mcprotocollib.protocol.packet.status.serverbound.ServerboundPingRequestPacket;
import org.geysermc.mcprotocollib.protocol.packet.ping.serverbound.ServerboundPingRequestPacket;
import org.geysermc.mcprotocollib.protocol.packet.status.serverbound.ServerboundStatusRequestPacket;

public class MinecraftCodec {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,10 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
helper.writeResourceLocation(out, this.channel);
out.writeBytes(this.data);
}

@Override
public boolean shouldRunOnGameThread() {
// GAME THREAD DETAIL: Only non-discarded payloads are handled async.
return false; // False, you need to handle making it async yourself
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,9 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
helper.writeString(out, entry.getValue());
}
}

@Override
public boolean shouldRunOnGameThread() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,4 @@ public ClientboundDisconnectPacket(ByteBuf in, MinecraftCodecHelper helper) {
public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
helper.writeComponent(out, this.reason);
}

@Override
public boolean isPriority() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,9 @@ public ClientboundPingPacket(ByteBuf in, MinecraftCodecHelper helper) {
public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
out.writeInt(this.id);
}

@Override
public boolean shouldRunOnGameThread() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,9 @@ public ClientboundResourcePackPopPacket(ByteBuf in, MinecraftCodecHelper helper)
public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
helper.writeNullable(out, this.id, helper::writeUUID);
}

@Override
public boolean shouldRunOnGameThread() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,9 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
out.writeBoolean(this.required);
helper.writeNullable(out, this.prompt, helper::writeComponent);
}

@Override
public boolean shouldRunOnGameThread() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,9 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
helper.writeString(out, link.link());
}
}

@Override
public boolean shouldRunOnGameThread() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,9 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
helper.writeResourceLocation(out, this.key);
helper.writeByteArray(out, this.payload);
}

@Override
public boolean shouldRunOnGameThread() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,10 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
helper.writeString(out, this.host);
helper.writeVarInt(out, this.port);
}

@Override
public boolean shouldRunOnGameThread() {
// GAME THREAD DETAIL: Code runs before packet is made async.
return false; // False, you need to handle making it async yourself
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,9 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
}
}
}

@Override
public boolean shouldRunOnGameThread() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,10 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
out.writeBoolean(allowsListing);
helper.writeVarInt(out, this.particleStatus.ordinal());
}

@Override
public boolean shouldRunOnGameThread() {
// GAME THREAD DETAIL: Code is only async during GAME state.
return false; // False, you need to handle making it async yourself
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,9 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
helper.writeUUID(out, id);
helper.writeVarInt(out, this.status.ordinal());
}

@Override
public boolean shouldRunOnGameThread() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ public ClientboundFinishConfigurationPacket(ByteBuf in, MinecraftCodecHelper hel
public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
}

@Override
public boolean shouldRunOnGameThread() {
return true;
}

@Override
public boolean isTerminal() {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,9 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
helper.writeNullable(buf, entry.getData(), helper::writeAnyTag);
});
}

@Override
public boolean shouldRunOnGameThread() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,9 @@ public void serialize(ByteBuf out, MinecraftCodecHelper helper) {
helper.writeString(buf, entry.getVersion());
});
}

@Override
public boolean shouldRunOnGameThread() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ public ServerboundFinishConfigurationPacket(ByteBuf in, MinecraftCodecHelper hel
public void serialize(ByteBuf buf, MinecraftCodecHelper helper) {
}

@Override
public boolean shouldRunOnGameThread() {
return true;
}

@Override
public boolean isTerminal() {
return true;
Expand Down
Loading
Loading