Char block null check (#1030)

**Add a null-check to CharBlocks FULL section layer-retrieval.**
 - It is possible to trim CharBlocks whilst it is attempting to read data due to the batching of chunks to help reduce memory
 - This is done when the number of chunks sitting loaded in memory with having been "submitted" to the queue for writing to disk becomes high
 - Seconday operations such as heightmap processing and lighting will quickly load chunks, meaning many chunks are submitted early
 - This leads to much higher chances of the chunk being submitted and subsequently trimmed given heightmap and light processing is done layer-by-layer over many chunks, rather than chunk-by-chunk - thus leading to NPEs.
 - By adding synchronisation to and around only the specific sections when loading/updating, and not blocking the whole chunk, many access can still be thread-safe without causing deadlocks
 - This allows removal of lots of the needless and very-slowing synchronisation on get**Block** methods

**Remove much of the synchronisation from ChunkHolder**
 - We shouldn't be synchronising with call() and safety should be added elsewhere. (plus it's making edits very very slow when queue target size is hit)
 - Also remove much of synchronisation because we've added the null-check and section-specific synchronisation to CharBlocks
 
**Some QOL/thread-safe data access changes**
- Replaces the Array#clone seen in the get blocks classes with System#arraycopy as deep cloning is not required, and is also slower than arraycopy
- Add System#arraycopy when accessing chunk section data via history to ensure it is not altered whilst being written
- Renaming EMPTY to empty means it is not implied to be a static variable

Fixes https://github.com/IntellectualSites/FastAsyncWorldEdit/issues/1028
Fixes https://github.com/IntellectualSites/FastAsyncWorldEdit/issues/1025
Fixes https://github.com/IntellectualSites/FastAsyncWorldEdit/issues/1089
Fixes https://github.com/IntellectualSites/FastAsyncWorldEdit/issues/1091
Fixes https://github.com/IntellectualSites/FastAsyncWorldEdit/issues/1097
This commit is contained in:
dordsor21
2021-06-08 15:28:16 +01:00
committed by GitHub
parent 1cd2d8d121
commit f6af9925e8
9 changed files with 197 additions and 115 deletions

View File

@ -349,17 +349,20 @@ public class BukkitGetBlocks_1_15_2 extends CharGetBlocks implements BukkitGetBl
};
}
private void updateGet(BukkitGetBlocks_1_15_2 get, Chunk nmsChunk, ChunkSection[] sections, ChunkSection section, char[] arr, int layer) {
private void updateGet(BukkitGetBlocks_1_15_2 get, Chunk nmsChunk, ChunkSection[] chunkSections, ChunkSection section, char[] arr, int layer) {
synchronized (get) {
if (this.getChunk() != nmsChunk) {
this.nmsChunk = nmsChunk;
this.sections = sections.clone();
this.sections = new ChunkSection[chunkSections.length];
System.arraycopy(chunkSections, 0, this.sections, 0, chunkSections.length);
this.reset();
}
if (this.sections == null) {
this.sections = sections.clone();
this.sections = new ChunkSection[chunkSections.length];
System.arraycopy(chunkSections, 0, this.sections, 0, chunkSections.length);
}
if (this.sections[layer] != section) {
// Not sure why it's funky, but it's what I did in commit fda7d00747abe97d7891b80ed8bb88d97e1c70d1 and I don't want to touch it >dords
this.sections[layer] = new ChunkSection[]{section}.clone()[0];
}
this.blocks[layer] = arr;
@ -421,9 +424,14 @@ public class BukkitGetBlocks_1_15_2 extends CharGetBlocks implements BukkitGetBl
bitMask |= 1 << layer;
char[] setArr = set.load(layer).clone();
char[] tmp = set.load(layer);
char[] setArr = new char[4096];
System.arraycopy(tmp, 0, setArr, 0, 4096);
if (createCopy) {
copy.storeSection(layer, loadPrivately(layer).clone());
char[] tmpLoad = loadPrivately(layer);
char[] copyArr = new char[4096];
System.arraycopy(tmpLoad, 0, copyArr, 0, 4096);
copy.storeSection(layer, copyArr);
}
ChunkSection newSection;
@ -457,7 +465,7 @@ public class BukkitGetBlocks_1_15_2 extends CharGetBlocks implements BukkitGetBl
} else if (existingSection != getSections(false)[layer]) {
this.sections[layer] = existingSection;
this.reset();
} else if (!Arrays.equals(update(layer, new char[4096]), loadPrivately(layer))) {
} else if (!Arrays.equals(update(layer, new char[4096], true), loadPrivately(layer))) {
this.reset(layer);
} else if (lock.isModified()) {
this.reset(layer);
@ -675,11 +683,13 @@ public class BukkitGetBlocks_1_15_2 extends CharGetBlocks implements BukkitGetBl
}
}
private char[] loadPrivately(int layer) {
if (super.sections[layer].isFull()) {
return super.blocks[layer];
private synchronized char[] loadPrivately(int layer) {
if (super.blocks[layer] != null) {
char[] blocks = new char[4096];
System.arraycopy(super.blocks[layer], 0, blocks, 0, 4096);
return blocks;
} else {
return BukkitGetBlocks_1_15_2.this.update(layer, null);
return BukkitGetBlocks_1_15_2.this.update(layer, null, true);
}
}
@ -689,8 +699,8 @@ public class BukkitGetBlocks_1_15_2 extends CharGetBlocks implements BukkitGetBl
}
@Override
public synchronized char[] update(int layer, char[] data) {
ChunkSection section = getSections(true)[layer];
public synchronized char[] update(int layer, char[] data, boolean aggressive) {
ChunkSection section = getSections(aggressive)[layer];
// Section is null, return empty array
if (section == null) {
data = new char[4096];
@ -807,15 +817,20 @@ public class BukkitGetBlocks_1_15_2 extends CharGetBlocks implements BukkitGetBl
public ChunkSection[] getSections(boolean force) {
if (force && forceLoadSections) {
return sections = getChunk().getSections().clone();
ChunkSection[] sections = getChunk().getSections();
ChunkSection[] copy = new ChunkSection[sections.length];
System.arraycopy(sections, 0, copy, 0, sections.length);
return copy;
}
ChunkSection[] tmp = sections;
if (tmp == null) {
synchronized (this) {
tmp = sections;
if (tmp == null) {
Chunk chunk = getChunk();
sections = tmp = chunk.getSections().clone();
ChunkSection[] chunkSections = getChunk().getSections();
tmp = new ChunkSection[chunkSections.length];
System.arraycopy(chunkSections, 0, tmp, 0, chunkSections.length);
sections = tmp;
}
}
}

View File

@ -349,17 +349,20 @@ public class BukkitGetBlocks_1_16_1 extends CharGetBlocks implements BukkitGetBl
};
}
private void updateGet(BukkitGetBlocks_1_16_1 get, Chunk nmsChunk, ChunkSection[] sections, ChunkSection section, char[] arr, int layer) {
private void updateGet(BukkitGetBlocks_1_16_1 get, Chunk nmsChunk, ChunkSection[] chunkSections, ChunkSection section, char[] arr, int layer) {
synchronized (get) {
if (this.getChunk() != nmsChunk) {
this.nmsChunk = nmsChunk;
this.sections = sections.clone();
this.sections = new ChunkSection[chunkSections.length];
System.arraycopy(chunkSections, 0, this.sections, 0, chunkSections.length);
this.reset();
}
if (this.sections == null) {
this.sections = sections.clone();
this.sections = new ChunkSection[chunkSections.length];
System.arraycopy(chunkSections, 0, this.sections, 0, chunkSections.length);
}
if (this.sections[layer] != section) {
// Not sure why it's funky, but it's what I did in commit fda7d00747abe97d7891b80ed8bb88d97e1c70d1 and I don't want to touch it >dords
this.sections[layer] = new ChunkSection[]{section}.clone()[0];
}
this.blocks[layer] = arr;
@ -421,9 +424,14 @@ public class BukkitGetBlocks_1_16_1 extends CharGetBlocks implements BukkitGetBl
bitMask |= 1 << layer;
char[] setArr = set.load(layer).clone();
char[] tmp = set.load(layer);
char[] setArr = new char[4096];
System.arraycopy(tmp, 0, setArr, 0, 4096);
if (createCopy) {
copy.storeSection(layer, loadPrivately(layer).clone());
char[] tmpLoad = loadPrivately(layer);
char[] copyArr = new char[4096];
System.arraycopy(tmpLoad, 0, copyArr, 0, 4096);
copy.storeSection(layer, copyArr);
}
ChunkSection newSection;
@ -457,7 +465,7 @@ public class BukkitGetBlocks_1_16_1 extends CharGetBlocks implements BukkitGetBl
} else if (existingSection != getSections(false)[layer]) {
this.sections[layer] = existingSection;
this.reset();
} else if (!Arrays.equals(update(layer, new char[4096]), loadPrivately(layer))) {
} else if (!Arrays.equals(update(layer, new char[4096], true), loadPrivately(layer))) {
this.reset(layer);
} else if (lock.isModified()) {
this.reset(layer);
@ -676,11 +684,13 @@ public class BukkitGetBlocks_1_16_1 extends CharGetBlocks implements BukkitGetBl
}
}
private char[] loadPrivately(int layer) {
if (super.sections[layer].isFull()) {
return super.blocks[layer];
private synchronized char[] loadPrivately(int layer) {
if (super.blocks[layer] != null) {
char[] blocks = new char[4096];
System.arraycopy(super.blocks[layer], 0, blocks, 0, 4096);
return blocks;
} else {
return BukkitGetBlocks_1_16_1.this.update(layer, null);
return BukkitGetBlocks_1_16_1.this.update(layer, null, true);
}
}
@ -690,8 +700,8 @@ public class BukkitGetBlocks_1_16_1 extends CharGetBlocks implements BukkitGetBl
}
@Override
public synchronized char[] update(int layer, char[] data) {
ChunkSection section = getSections(true)[layer];
public synchronized char[] update(int layer, char[] data, boolean aggressive) {
ChunkSection section = getSections(aggressive)[layer];
// Section is null, return empty array
if (section == null) {
data = new char[4096];
@ -808,15 +818,20 @@ public class BukkitGetBlocks_1_16_1 extends CharGetBlocks implements BukkitGetBl
public ChunkSection[] getSections(boolean force) {
if (force && forceLoadSections) {
return sections = getChunk().getSections().clone();
ChunkSection[] sections = getChunk().getSections();
ChunkSection[] copy = new ChunkSection[sections.length];
System.arraycopy(sections, 0, copy, 0, sections.length);
return copy;
}
ChunkSection[] tmp = sections;
if (tmp == null) {
synchronized (this) {
tmp = sections;
if (tmp == null) {
Chunk chunk = getChunk();
sections = tmp = chunk.getSections().clone();
ChunkSection[] chunkSections = getChunk().getSections();
tmp = new ChunkSection[chunkSections.length];
System.arraycopy(chunkSections, 0, tmp, 0, chunkSections.length);
sections = tmp;
}
}
}

View File

@ -352,17 +352,20 @@ public class BukkitGetBlocks_1_16_2 extends CharGetBlocks implements BukkitGetBl
};
}
private void updateGet(BukkitGetBlocks_1_16_2 get, Chunk nmsChunk, ChunkSection[] sections, ChunkSection section, char[] arr, int layer) {
private void updateGet(BukkitGetBlocks_1_16_2 get, Chunk nmsChunk, ChunkSection[] chunkSections, ChunkSection section, char[] arr, int layer) {
synchronized (get) {
if (this.getChunk() != nmsChunk) {
this.nmsChunk = nmsChunk;
this.sections = sections.clone();
this.sections = new ChunkSection[chunkSections.length];
System.arraycopy(chunkSections, 0, this.sections, 0, chunkSections.length);
this.reset();
}
if (this.sections == null) {
this.sections = sections.clone();
this.sections = new ChunkSection[chunkSections.length];
System.arraycopy(chunkSections, 0, this.sections, 0, chunkSections.length);
}
if (this.sections[layer] != section) {
// Not sure why it's funky, but it's what I did in commit fda7d00747abe97d7891b80ed8bb88d97e1c70d1 and I don't want to touch it >dords
this.sections[layer] = new ChunkSection[]{section}.clone()[0];
}
this.blocks[layer] = arr;
@ -424,9 +427,14 @@ public class BukkitGetBlocks_1_16_2 extends CharGetBlocks implements BukkitGetBl
bitMask |= 1 << layer;
char[] setArr = set.load(layer).clone();
char[] tmp = set.load(layer);
char[] setArr = new char[4096];
System.arraycopy(tmp, 0, setArr, 0, 4096);
if (createCopy) {
copy.storeSection(layer, loadPrivately(layer).clone());
char[] tmpLoad = loadPrivately(layer);
char[] copyArr = new char[4096];
System.arraycopy(tmpLoad, 0, copyArr, 0, 4096);
copy.storeSection(layer, copyArr);
}
ChunkSection newSection;
@ -460,7 +468,7 @@ public class BukkitGetBlocks_1_16_2 extends CharGetBlocks implements BukkitGetBl
} else if (existingSection != getSections(false)[layer]) {
this.sections[layer] = existingSection;
this.reset();
} else if (!Arrays.equals(update(layer, new char[4096]), loadPrivately(layer))) {
} else if (!Arrays.equals(update(layer, new char[4096], true), loadPrivately(layer))) {
this.reset(layer);
} else if (lock.isModified()) {
this.reset(layer);
@ -679,11 +687,13 @@ public class BukkitGetBlocks_1_16_2 extends CharGetBlocks implements BukkitGetBl
}
}
private char[] loadPrivately(int layer) {
if (super.sections[layer].isFull()) {
return super.blocks[layer];
private synchronized char[] loadPrivately(int layer) {
if (super.blocks[layer] != null) {
char[] blocks = new char[4096];
System.arraycopy(super.blocks[layer], 0, blocks, 0, 4096);
return blocks;
} else {
return BukkitGetBlocks_1_16_2.this.update(layer, null);
return BukkitGetBlocks_1_16_2.this.update(layer, null, true);
}
}
@ -693,8 +703,8 @@ public class BukkitGetBlocks_1_16_2 extends CharGetBlocks implements BukkitGetBl
}
@Override
public synchronized char[] update(int layer, char[] data) {
ChunkSection section = getSections(true)[layer];
public synchronized char[] update(int layer, char[] data, boolean aggressive) {
ChunkSection section = getSections(aggressive)[layer];
// Section is null, return empty array
if (section == null) {
data = new char[4096];
@ -811,15 +821,20 @@ public class BukkitGetBlocks_1_16_2 extends CharGetBlocks implements BukkitGetBl
public ChunkSection[] getSections(boolean force) {
if (force && forceLoadSections) {
return sections = getChunk().getSections().clone();
ChunkSection[] sections = getChunk().getSections();
ChunkSection[] copy = new ChunkSection[sections.length];
System.arraycopy(sections, 0, copy, 0, sections.length);
return copy;
}
ChunkSection[] tmp = sections;
if (tmp == null) {
synchronized (this) {
tmp = sections;
if (tmp == null) {
Chunk chunk = getChunk();
sections = tmp = chunk.getSections().clone();
ChunkSection[] chunkSections = getChunk().getSections();
tmp = new ChunkSection[chunkSections.length];
System.arraycopy(chunkSections, 0, tmp, 0, chunkSections.length);
sections = tmp;
}
}
}

View File

@ -352,17 +352,20 @@ public class BukkitGetBlocks_1_16_5 extends CharGetBlocks implements BukkitGetBl
};
}
private void updateGet(BukkitGetBlocks_1_16_5 get, Chunk nmsChunk, ChunkSection[] sections, ChunkSection section, char[] arr, int layer) {
private void updateGet(BukkitGetBlocks_1_16_5 get, Chunk nmsChunk, ChunkSection[] chunkSections, ChunkSection section, char[] arr, int layer) {
synchronized (get) {
if (this.getChunk() != nmsChunk) {
this.nmsChunk = nmsChunk;
this.sections = sections.clone();
this.sections = new ChunkSection[chunkSections.length];
System.arraycopy(chunkSections, 0, this.sections, 0, chunkSections.length);
this.reset();
}
if (this.sections == null) {
this.sections = sections.clone();
this.sections = new ChunkSection[chunkSections.length];
System.arraycopy(chunkSections, 0, this.sections, 0, chunkSections.length);
}
if (this.sections[layer] != section) {
// Not sure why it's funky, but it's what I did in commit fda7d00747abe97d7891b80ed8bb88d97e1c70d1 and I don't want to touch it >dords
this.sections[layer] = new ChunkSection[]{section}.clone()[0];
}
this.blocks[layer] = arr;
@ -424,9 +427,14 @@ public class BukkitGetBlocks_1_16_5 extends CharGetBlocks implements BukkitGetBl
bitMask |= 1 << layer;
char[] setArr = set.load(layer).clone();
char[] tmp = set.load(layer);
char[] setArr = new char[4096];
System.arraycopy(tmp, 0, setArr, 0, 4096);
if (createCopy) {
copy.storeSection(layer, loadPrivately(layer).clone());
char[] tmpLoad = loadPrivately(layer);
char[] copyArr = new char[4096];
System.arraycopy(tmpLoad, 0, copyArr, 0, 4096);
copy.storeSection(layer, copyArr);
}
ChunkSection newSection;
@ -460,7 +468,7 @@ public class BukkitGetBlocks_1_16_5 extends CharGetBlocks implements BukkitGetBl
} else if (existingSection != getSections(false)[layer]) {
this.sections[layer] = existingSection;
this.reset();
} else if (!Arrays.equals(update(layer, new char[4096]), loadPrivately(layer))) {
} else if (!Arrays.equals(update(layer, new char[4096], true), loadPrivately(layer))) {
this.reset(layer);
} else if (lock.isModified()) {
this.reset(layer);
@ -679,11 +687,13 @@ public class BukkitGetBlocks_1_16_5 extends CharGetBlocks implements BukkitGetBl
}
}
private char[] loadPrivately(int layer) {
if (super.sections[layer].isFull()) {
return super.blocks[layer];
private synchronized char[] loadPrivately(int layer) {
if (super.blocks[layer] != null) {
char[] blocks = new char[4096];
System.arraycopy(super.blocks[layer], 0, blocks, 0, 4096);
return blocks;
} else {
return BukkitGetBlocks_1_16_5.this.update(layer, null);
return BukkitGetBlocks_1_16_5.this.update(layer, null, false);
}
}
@ -693,8 +703,8 @@ public class BukkitGetBlocks_1_16_5 extends CharGetBlocks implements BukkitGetBl
}
@Override
public synchronized char[] update(int layer, char[] data) {
ChunkSection section = getSections(true)[layer];
public synchronized char[] update(int layer, char[] data, boolean aggressive) {
ChunkSection section = getSections(aggressive)[layer];
// Section is null, return empty array
if (section == null) {
data = new char[4096];
@ -811,15 +821,20 @@ public class BukkitGetBlocks_1_16_5 extends CharGetBlocks implements BukkitGetBl
public ChunkSection[] getSections(boolean force) {
if (force && forceLoadSections) {
return sections = getChunk().getSections().clone();
ChunkSection[] sections = getChunk().getSections();
ChunkSection[] copy = new ChunkSection[sections.length];
System.arraycopy(sections, 0, copy, 0, sections.length);
return copy;
}
ChunkSection[] tmp = sections;
if (tmp == null) {
synchronized (this) {
tmp = sections;
if (tmp == null) {
Chunk chunk = getChunk();
sections = tmp = chunk.getSections().clone();
ChunkSection[] chunkSections = getChunk().getSections();
tmp = new ChunkSection[chunkSections.length];
System.arraycopy(chunkSections, 0, tmp, 0, chunkSections.length);
sections = tmp;
}
}
}