User Storage System #886
User Storage System #886
Conversation
Added a system that allows developers to associate a certain value with the PacketEvent User class, making it easier and faster to identify the user on any PacketEvent at all times. MethodHandle was used instead of VarHandle, as I found it to be slightly faster, with, and without warmup calls. Here's the benchmark with 1000 warmup calls and 10000 evaluation calls: https://paste.md-5.net/
api/src/main/java/com/github/retrooper/packetevents/event/EventManager.java
Outdated
Show resolved
Hide resolved
api/src/main/java/com/github/retrooper/packetevents/protocol/player/User.java
Outdated
Show resolved
Hide resolved
api/src/main/java/com/github/retrooper/packetevents/protocol/player/User.java
Outdated
Show resolved
Hide resolved
|
|
||
| private Object get0(int index) { | ||
| if (index >= data.length) { | ||
| this.regrow(); |
There was a problem hiding this comment.
why are you growing here?
There was a problem hiding this comment.
On any data access, be it write or read, if the class that creates the identifier is initialized later on the initial data size won't be enough. The grow happens only when we're implicitly told that this has happened. It's also safe to do this, since the id is provided internally only
| //It was found to be slightly faster than VarHandles with excessive amount of calls | ||
| private static final MethodHandle setElement = MethodHandles.arrayElementSetter(Object[].class);//array, index, value | ||
| private static final MethodHandle getElement = MethodHandles.arrayElementGetter(Object[].class);//array, index | ||
| private static final Map<Object, Integer> pluginToId = new ConcurrentHashMap<>(2); |
There was a problem hiding this comment.
if the keys here are plugin instances, this should definetely be a WeakHashMap
otherwhise this will cause memory leaks when a plugin gets disabled or unloaded
because of this you will need to do manual synchronization, see some coment below for mor info
There was a problem hiding this comment.
im also not sure if its a good idea to key these ids by "plugin" instance - in my opinion it makes more sense to key the id map by the class of the identifier type, so you can create multiple keys per plugin
There was a problem hiding this comment.
if the keys here are plugin instances, this should definetely be a
WeakHashMapotherwhise this will cause memory leaks when a plugin gets disabled or unloadedbecause of this you will need to do manual synchronization, see some coment below for mor info
Guava's MapMaker is the best solution then
There was a problem hiding this comment.
im also not sure if its a good idea to key these ids by "plugin" instance - in my opinion it makes more sense to key the id map by the class of the identifier type, so you can create multiple keys per plugin
The main idea behind this is that you create one key, which holds one custom object, that can encapsulate all the other values associated. Otherwise this could lead to memory being wasted
...src/main/java/com/github/retrooper/packetevents/protocol/player/storage/FastUserStorage.java
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,26 @@ | |||
| package com.github.retrooper.packetevents.protocol.player.storage; | |||
|
|
|||
| public final class StorageValueId extends StorageIdBase { | |||
There was a problem hiding this comment.
is there any reason for this to exist?
There was a problem hiding this comment.
Can be useful for example if a developer wants to store different types of data throught different parts of joining, where the automatic casting would be only an obstacle
|
This won't be very useful unless only 1 instance of PE is used. Channel attributes are more useful for that. |
Added a system that allows developers to associate a certain value with the PacketEvent User class, making it easier and faster to identify the user on any PacketEvent at all times. MethodHandle was used instead of VarHandle, as I found it to be slightly faster, with, and without warmup calls. Here's the benchmark with 1000 warmup calls and 10000 evaluation calls: https://paste.md-5.net/exedugixog.cpp