[Question] Why does NBTAPI use read-write locks?
BlackBaroness opened this issue ยท 2 comments
I noticed that compounds use locks under the hood, but why? This doesn't make accessing items async safe, because these locks are local for specific NBT session. Who would share their temporary NBT objects between threads?
AFAIK, locking is generally expensive, especially locks (not synchronized
), especially ReentrantReadWriteLock
:
ReentrantReadWriteLocks can be used to improve concurrency in some uses of some kinds of Collections. This is typically worthwhile only when the collections are expected to be large, accessed by more reader threads than writer threads, and entail operations with overhead that outweighs synchronization overhead.
From javadoc
I used NBTAPI many times, and I still can't imagine a scenario where I would share my ReadWriteNBT
object across the threads. So why do we spend CPU time to use locks?
ReentrantReadWriteLock are rather cheap enough for this use-case (especially at the amount the nbtapi is used per tick/second, no one is doing 10s or 100s of thousands of calls per second). The locks are pre ReadWriteNBT and were the result of people (probably not knowing the ramifications) reading/writing data on NbtCompounds multithreaded without any protection(probably NBTFiles), causing ConcurrentModificationExceptions.
So in the end it's just there to make the API more noob-friendly. If I wanted to optimize the CPU time more I'd rather go for minimizing reflections/compiling handlers at runtime and extracting the internal Maps/Lists of compounds.
During 2.12.0 I did some benchmarking with the new methods (https://modrinth.com/plugin/nbtapi/version/2.12.0-RC1 ) and with the locks in there on NMS Items got over 4 million reads and 1 million writes per second.
It would be possible to make the locking optional, but as it doesn't have any real-world impact, doubt that's worth it.