Perf/optimize packet listener#252
Merged
twisti-dev merged 3 commits intoversion/1.21.11from Mar 17, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors and hardens the Bukkit packet listener pipeline to better support early-connection packet handling, improve thread-safety/performance of listener dispatch, and tighten API null-safety contracts.
Changes:
- Refactored channel injection to store a per-channel
PacketHandlervia NettyAttributeKey, and ensured injector cleanup on plugin disable. - Reworked
SurfBukkitPacketListenerApiImpllistener registration/invocation to use concurrent collections and normalizedMethodHandleinvokers. - Added early-phase clientbound handling API (
handleEarlyClientboundPacket), added@NullMarked/status annotations, and added exception guards around early handler invocations.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| surf-api-bukkit/surf-api-bukkit-server/.../packet/listener/PlayerChannelInjector.kt | Replaces cache-based injection with per-channel attribute-based handler management; adds safer login-phase setup and unified packet handling path. |
| surf-api-bukkit/surf-api-bukkit-server/.../packet/PacketApiLoader.kt | Ensures PlayerChannelInjector is unregistered during plugin disable to prevent lingering hooks. |
| surf-api-bukkit/surf-api-bukkit-server/.../SurfBukkitPacketListenerApiImpl.kt | Refactors listener storage/invocation with ConcurrentHashMap + CopyOnWriteArraySet and MethodHandle normalization. |
| surf-api-bukkit/surf-api-bukkit-server/.../SurfBukkitNmsBridgeImpl.kt | Wraps early packet listener calls with try/catch and continues on failure to protect the packet pipeline. |
| surf-api-bukkit/surf-api-bukkit-api/.../NmsServerboundPacketListener.java | Marks nullness defaults and makes early serverbound handling explicit by throwing if no Player exists unless overridden. |
| surf-api-bukkit/surf-api-bukkit-api/.../NmsClientboundPacketListener.java | Introduces handleEarlyClientboundPacket with explicit opt-in semantics for pre-Player phases. |
| surf-api-bukkit/surf-api-bukkit-api/.../NmsPacketListener.java | Adds internal/status annotations and @NullMarked to improve null-safety signaling. |
| surf-api-bukkit/surf-api-bukkit-api/api/surf-api-bukkit-api.api | Updates API surface snapshot to include the new early clientbound handler. |
| gradle.properties | Bumps project version to 1.21.11-2.69.0. |
| } | ||
|
|
||
| Packet::class.java.isAssignableFrom(returnType) -> ListenerResultConverter<Packet<*>?> { result, original -> | ||
| result |
| private val injectedChannels = Caffeine.newBuilder() | ||
| .weakKeys() | ||
| .build<Channel, Boolean>() | ||
| private val packetHandlerKey = AttributeKey.newInstance<PacketHandler>("surf_api_packet_handler") |
| } | ||
|
|
||
| private fun createResultConverter(returnType: Class<*>): ListenerResultConverter<*> = when { | ||
| returnType == Void.TYPE -> ListenerResultConverter<Unit> { _, p -> p } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces significant improvements and refactoring to the packet listener infrastructure in the Bukkit API, focusing on safer handling of early connection phases, improved listener registration, and modernization of concurrency primitives. The changes enhance robustness, flexibility, and maintainability for handling Minecraft packets in various connection states.
API and Listener Handling Improvements
handleEarlyClientboundPacketmethod toNmsClientboundPacketListener, mirroring the serverbound variant, and updated both to throw an exception if called before aPlayeris available, requiring explicit override for pre-login handling. This ensures listeners are aware of early-phase packet handling and must opt-in if needed. [1] [2] [3]NmsClientboundPacketListener,NmsServerboundPacketListener,NmsPacketListener) with@NullMarkedand improved API status annotations for better null-safety and documentation. [1] [2] [3]Exception Handling and Logging
SurfBukkitNmsBridgeImpl) to wrap early packet handler invocations in try-catch blocks, logging severe errors and ensuring packet flow continuity by returningPacketListenerResult.CONTINUEon exceptions. [1] [2]Listener Registration and Invocation Refactor
SurfBukkitPacketListenerApiImplto useConcurrentHashMapandCopyOnWriteArraySetfor thread-safe listener management, replaced reflective method invocation with normalizedMethodHandleinvokers, and unified support for bothServerPlayerand BukkitPlayerparameters in listener methods. This includes improved error handling and parameter validation during registration. [1] [2]callListenerlogic in favor of a more robustListenerInvokerandListenerResultConvertersystem, streamlining the invocation process and supporting flexible return types (void,PacketListenerResult, orPacket).Miscellaneous
PlayerChannelInjectoron plugin disable and refactored imports for clarity and correctness in related files. [1] [2]1.21.11-2.68.0to1.21.11-2.69.0.