#971: Remove strong chunk reference in PDC

A previous fix for SPIGOT-6814 implemented a callback function for the
PDC implementation that could be set to actively define a chunk as
unsaved, allowing chunks that have not been mutated through block
changes to still require saving if the chunks pdc was mutated.

This implementation however would pass a callback that references the
chunk access internally, meaning the PDC now actively holds onto a
callback that holds a reference to the entire chunk.

Aditionally, this change also impacted the pdc for item metas and
entities for really no reason whatsoever.

This commit re-implements the fix by introducing a new child of the pdc
implementation that the chunk now uses as its pdc. This specific
implementation maintains a dirty flag that is set to `true` on any form
of mutation and set back to false by the chunk that owns the PDC
whenever the chunk itself is flag as no longer dirty.

By: Bjarne Koll <lynxplay101@gmail.com>
This commit is contained in:
CraftBukkit/Spigot 2021-12-05 08:52:51 +11:00
parent 14215fdadd
commit 24b8f3c641
4 changed files with 88 additions and 35 deletions

View File

@ -39,19 +39,18 @@
public Chunk(WorldServer worldserver, ProtoChunk protochunk, @Nullable Chunk.c chunk_c) {
this(worldserver, protochunk.getPos(), protochunk.getUpgradeData(), protochunk.unpackBlockTicks(), protochunk.unpackFluidTicks(), protochunk.getInhabitedTime(), protochunk.getSections(), chunk_c, protochunk.getBlendingData());
Iterator iterator = protochunk.getBlockEntities().values().iterator();
@@ -142,6 +154,11 @@
@@ -142,6 +154,10 @@
this.setLightCorrect(protochunk.isLightCorrect());
this.unsaved = true;
+ this.needsDecoration = true; // CraftBukkit
+ // CraftBukkit start
+ this.persistentDataContainer = protochunk.persistentDataContainer; // SPIGOT-6814: copy PDC to account for 1.17 to 1.18 chunk upgrading.
+ this.persistentDataContainer.setCallback(() -> setUnsaved(true)); // SPIGOT-6814: Handle cases were only persistentData is saved
+ // CraftBukkit end
}
@Override
@@ -238,9 +255,16 @@
@@ -238,9 +254,16 @@
}
}
@ -68,7 +67,7 @@
int i = blockposition.getY();
ChunkSection chunksection = this.getSection(this.getSectionIndex(i));
boolean flag1 = chunksection.hasOnlyAir();
@@ -279,7 +303,8 @@
@@ -279,7 +302,8 @@
if (!chunksection.getBlockState(j, k, l).is(block)) {
return null;
} else {
@ -78,7 +77,7 @@
iblockdata.onPlace(this.level, blockposition, iblockdata1, flag);
}
@@ -324,7 +349,12 @@
@@ -324,7 +348,12 @@
@Nullable
public TileEntity getBlockEntity(BlockPosition blockposition, Chunk.EnumTileEntityState chunk_enumtileentitystate) {
@ -92,7 +91,7 @@
if (tileentity == null) {
NBTTagCompound nbttagcompound = (NBTTagCompound) this.pendingBlockEntities.remove(blockposition);
@@ -395,6 +425,13 @@
@@ -395,6 +424,13 @@
tileentity1.setRemoved();
}
@ -106,7 +105,7 @@
}
}
@@ -424,6 +461,12 @@
@@ -424,6 +460,12 @@
if (this.isInLevel()) {
TileEntity tileentity = (TileEntity) this.blockEntities.remove(blockposition);
@ -119,7 +118,7 @@
if (tileentity != null) {
this.removeGameEventListener(tileentity);
tileentity.setRemoved();
@@ -471,6 +514,55 @@
@@ -471,6 +513,55 @@
}
@ -175,7 +174,7 @@
public boolean isEmpty() {
return false;
}
@@ -659,7 +751,7 @@
@@ -659,7 +750,7 @@
private <T extends TileEntity> void updateBlockEntityTicker(T t0) {
IBlockData iblockdata = t0.getBlockState();
@ -184,7 +183,7 @@
if (blockentityticker == null) {
this.removeBlockEntityTicker(t0.getBlockPos());
@@ -752,7 +844,7 @@
@@ -752,7 +843,7 @@
private boolean loggedInvalidBlockState;
a(TileEntity tileentity, BlockEntityTicker blockentityticker) {
@ -193,7 +192,7 @@
this.ticker = blockentityticker;
}
@@ -775,7 +867,7 @@
@@ -775,7 +866,7 @@
this.loggedInvalidBlockState = true;
Chunk.LOGGER.warn("Block entity {} @ {} state {} invalid for ticking:", new org.apache.logging.log4j.util.Supplier[]{this::getType, this::getPos, () -> {
return iblockdata;

View File

@ -6,25 +6,37 @@
+ // CraftBukkit start - SPIGOT-6814: move to IChunkAccess to account for 1.17 to 1.18 chunk upgrading.
+ private static final org.bukkit.craftbukkit.persistence.CraftPersistentDataTypeRegistry DATA_TYPE_REGISTRY = new org.bukkit.craftbukkit.persistence.CraftPersistentDataTypeRegistry();
+ public org.bukkit.craftbukkit.persistence.CraftPersistentDataContainer persistentDataContainer = new org.bukkit.craftbukkit.persistence.CraftPersistentDataContainer(DATA_TYPE_REGISTRY);
+ public org.bukkit.craftbukkit.persistence.DirtyCraftPersistentDataContainer persistentDataContainer = new org.bukkit.craftbukkit.persistence.DirtyCraftPersistentDataContainer(DATA_TYPE_REGISTRY);
+ // CraftBukkit end
+
public IChunkAccess(ChunkCoordIntPair chunkcoordintpair, ChunkConverter chunkconverter, LevelHeightAccessor levelheightaccessor, IRegistry<BiomeBase> iregistry, long i, @Nullable ChunkSection[] achunksection, @Nullable BlendingData blendingdata) {
this.chunkPos = chunkcoordintpair;
this.upgradeData = chunkconverter;
@@ -94,7 +99,12 @@
@@ -94,7 +99,11 @@
}
replaceMissingSections(levelheightaccessor, iregistry, this.sections);
+ // CraftBukkit start
+ this.biomeRegistry = iregistry;
+ this.persistentDataContainer.setCallback(() -> setUnsaved(true)); // CraftBukkit - SPIGOT-6814: Handle cases were only persistentData is saved
}
+ public final IRegistry<BiomeBase> biomeRegistry;
+ // CraftBukkit end
private static void replaceMissingSections(LevelHeightAccessor levelheightaccessor, IRegistry<BiomeBase> iregistry, ChunkSection[] achunksection) {
for (int i = 0; i < achunksection.length; ++i) {
@@ -258,10 +267,11 @@
public void setUnsaved(boolean flag) {
this.unsaved = flag;
+ if (!flag) this.persistentDataContainer.dirty(false); // CraftBukkit - SPIGOT-6814: chunk was saved, pdc is no longer dirty
}
public boolean isUnsaved() {
- return this.unsaved;
+ return this.unsaved || this.persistentDataContainer.dirty(); // CraftBukkit - SPIGOT-6814: chunk is unsaved if pdc was mutated
}
public abstract ChunkStatus getStatus();
@@ -394,6 +404,27 @@
}
}

View File

@ -15,13 +15,11 @@ import org.bukkit.persistence.PersistentDataAdapterContext;
import org.bukkit.persistence.PersistentDataContainer;
import org.bukkit.persistence.PersistentDataType;
public final class CraftPersistentDataContainer implements PersistentDataContainer {
public class CraftPersistentDataContainer implements PersistentDataContainer {
private static final Callback EMPTY = () -> { };
private final Map<String, NBTBase> customDataTags = new HashMap<>();
private final CraftPersistentDataTypeRegistry registry;
private final CraftPersistentDataAdapterContext adapterContext;
private Callback callback = EMPTY;
public CraftPersistentDataContainer(Map<String, NBTBase> customTags, CraftPersistentDataTypeRegistry registry) {
this(registry);
@ -33,14 +31,6 @@ public final class CraftPersistentDataContainer implements PersistentDataContain
this.adapterContext = new CraftPersistentDataAdapterContext(this.registry);
}
public void setCallback(Callback callback) {
if (callback == null) {
this.callback = EMPTY;
return;
}
this.callback = callback;
}
@Override
public <T, Z> void set(NamespacedKey key, PersistentDataType<T, Z> type, Z value) {
@ -49,7 +39,6 @@ public final class CraftPersistentDataContainer implements PersistentDataContain
Validate.notNull(value, "The provided value for the custom value was null");
this.customDataTags.put(key.toString(), registry.wrap(type.getPrimitiveType(), type.toPrimitive(value, adapterContext)));
callback.onValueChange();
}
@Override
@ -103,7 +92,6 @@ public final class CraftPersistentDataContainer implements PersistentDataContain
Validate.notNull(key, "The provided key for the custom value was null");
this.customDataTags.remove(key.toString());
callback.onValueChange();
}
@Override
@ -138,19 +126,16 @@ public final class CraftPersistentDataContainer implements PersistentDataContain
public void put(String key, NBTBase base) {
this.customDataTags.put(key, base);
callback.onValueChange();
}
public void putAll(Map<String, NBTBase> map) {
this.customDataTags.putAll(map);
callback.onValueChange();
}
public void putAll(NBTTagCompound compound) {
for (String key : compound.getAllKeys()) {
this.customDataTags.put(key, compound.get(key));
}
callback.onValueChange();
}
public Map<String, NBTBase> getRaw() {
@ -171,9 +156,4 @@ public final class CraftPersistentDataContainer implements PersistentDataContain
public Map<String, Object> serialize() {
return (Map<String, Object>) CraftNBTTagConfigSerializer.serialize(toTagCompound());
}
@FunctionalInterface
public interface Callback {
void onValueChange();
}
}

View File

@ -0,0 +1,62 @@
package org.bukkit.craftbukkit.persistence;
import java.util.Map;
import net.minecraft.nbt.NBTBase;
import net.minecraft.nbt.NBTTagCompound;
import org.bukkit.NamespacedKey;
import org.bukkit.persistence.PersistentDataType;
/**
* A child class of the persistent data container that recalls if it has been
* mutated from an external caller.
*/
public final class DirtyCraftPersistentDataContainer extends CraftPersistentDataContainer {
private boolean dirty;
public DirtyCraftPersistentDataContainer(Map<String, NBTBase> customTags, CraftPersistentDataTypeRegistry registry) {
super(customTags, registry);
}
public DirtyCraftPersistentDataContainer(CraftPersistentDataTypeRegistry registry) {
super(registry);
}
public boolean dirty() {
return this.dirty;
}
public void dirty(final boolean dirty) {
this.dirty = dirty;
}
@Override
public <T, Z> void set(NamespacedKey key, PersistentDataType<T, Z> type, Z value) {
super.set(key, type, value);
this.dirty(true);
}
@Override
public void remove(NamespacedKey key) {
super.remove(key);
this.dirty(true);
}
@Override
public void put(String key, NBTBase base) {
super.put(key, base);
this.dirty(true);
}
@Override
public void putAll(NBTTagCompound compound) {
super.putAll(compound);
this.dirty(true);
}
@Override
public void putAll(Map<String, NBTBase> map) {
super.putAll(map);
this.dirty(true);
}
}