Advisory chunk locking for chunk population (#4513)

this allows chunks locked for population to be modified. If the PopulationTask detects that the chunk was modified during the onCompletion(), the result of the population will be discarded and rescheduled, so that it includes user modifications.
This commit is contained in:
Dylan T 2021-10-31 21:11:20 +00:00 committed by GitHub
parent f1a791ef75
commit 0f78a2b5ef
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 129 additions and 35 deletions

38
src/world/ChunkLockId.php Normal file
View File

@ -0,0 +1,38 @@
<?php
/*
*
* ____ _ _ __ __ _ __ __ ____
* | _ \ ___ ___| | _____| |_| \/ (_)_ __ ___ | \/ | _ \
* | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
* | __/ (_) | (__| < __/ |_| | | | | | | | __/_____| | | | __/
* |_| \___/ \___|_|\_\___|\__|_| |_|_|_| |_|\___| |_| |_|_|
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* @author PocketMine Team
* @link http://www.pocketmine.net/
*
*
*/
declare(strict_types=1);
namespace pocketmine\world;
use pocketmine\utils\NotCloneable;
use pocketmine\utils\NotSerializable;
/**
* Represents a unique lock ID for use with World chunk locking.
*
* @see World::lockChunk()
* @see World::unlockChunk()
*/
final class ChunkLockId{
use NotCloneable;
use NotSerializable;
}

View File

@ -249,7 +249,7 @@ class World implements ChunkManager{
/** @var bool[] */
private $activeChunkPopulationTasks = [];
/** @var bool[] */
/** @var ChunkLockId[] */
private $chunkLock = [];
/** @var int */
private $maxConcurrentChunkPopulationTasks = 2;
@ -1588,15 +1588,14 @@ class World implements ChunkManager{
}
$chunkX = $x >> Chunk::COORD_BIT_SIZE;
$chunkZ = $z >> Chunk::COORD_BIT_SIZE;
if($this->isChunkLocked($chunkX, $chunkZ)){
throw new WorldException("Terrain is locked for generation/population");
}
if($this->loadChunk($chunkX, $chunkZ) === null){ //current expected behaviour is to try to load the terrain synchronously
throw new WorldException("Cannot set a block in un-generated terrain");
}
$this->timings->setBlock->startTiming();
$this->unlockChunk($chunkX, $chunkZ, null);
$oldBlock = $this->getBlockAt($x, $y, $z, true, false);
$block = clone $block;
@ -2046,10 +2045,7 @@ class World implements ChunkManager{
public function setBiomeId(int $x, int $z, int $biomeId) : void{
$chunkX = $x >> Chunk::COORD_BIT_SIZE;
$chunkZ = $z >> Chunk::COORD_BIT_SIZE;
if($this->isChunkLocked($chunkX, $chunkZ)){
//the changes would be overwritten when the generation finishes
throw new WorldException("Chunk is currently locked for async generation/population");
}
$this->unlockChunk($chunkX, $chunkZ, null);
if(($chunk = $this->loadChunk($chunkX, $chunkZ)) !== null){
$chunk->setBiomeId($x & Chunk::COORD_MASK, $z & Chunk::COORD_MASK, $biomeId);
}else{
@ -2103,18 +2099,50 @@ class World implements ChunkManager{
return $result;
}
public function lockChunk(int $chunkX, int $chunkZ) : void{
/**
* Flags a chunk as locked, usually for async modification.
*
* This is an **advisory lock**. This means that the lock does **not** prevent the chunk from being modified on the
* main thread, such as by setBlock() or setBiomeId(). However, you can use it to detect when such modifications
* have taken place - unlockChunk() with the same lockID will fail and return false if this happens.
*
* This is used internally by the generation system to ensure that two PopulationTasks don't try to modify the same
* chunk at the same time. Generation will respect these locks and won't try to do generation of chunks over which
* a lock is held.
*
* WARNING: Be sure to release all locks once you're done with them, or you WILL have problems with terrain not
* being generated.
*/
public function lockChunk(int $chunkX, int $chunkZ, ChunkLockId $lockId) : void{
$chunkHash = World::chunkHash($chunkX, $chunkZ);
if(isset($this->chunkLock[$chunkHash])){
throw new \InvalidArgumentException("Chunk $chunkX $chunkZ is already locked");
}
$this->chunkLock[$chunkHash] = true;
$this->chunkLock[$chunkHash] = $lockId;
}
public function unlockChunk(int $chunkX, int $chunkZ) : void{
unset($this->chunkLock[World::chunkHash($chunkX, $chunkZ)]);
/**
* Unlocks a chunk previously locked by lockChunk().
*
* You must provide the same lockID as provided to lockChunk().
* If a null lockID is given, any existing lock will be removed from the chunk, regardless of who owns it.
*
* Returns true if unlocking was successful, false otherwise.
*/
public function unlockChunk(int $chunkX, int $chunkZ, ?ChunkLockId $lockId) : bool{
$chunkHash = World::chunkHash($chunkX, $chunkZ);
if(isset($this->chunkLock[$chunkHash]) && ($lockId === null || $this->chunkLock[$chunkHash] === $lockId)){
unset($this->chunkLock[$chunkHash]);
return true;
}
return false;
}
/**
* Returns whether anyone currently has a lock on the chunk at the given coordinates.
* You should check this to make sure that population tasks aren't currently modifying chunks that you want to use
* in async tasks.
*/
public function isChunkLocked(int $chunkX, int $chunkZ) : bool{
return isset($this->chunkLock[World::chunkHash($chunkX, $chunkZ)]);
}
@ -2814,16 +2842,18 @@ class World implements ChunkManager{
$this->chunkPopulationRequestMap[$index] = $resolver;
}
$chunkPopulationLockId = new ChunkLockId();
for($xx = -1; $xx <= 1; ++$xx){
for($zz = -1; $zz <= 1; ++$zz){
$this->lockChunk($x + $xx, $z + $zz);
$this->lockChunk($x + $xx, $z + $zz, $chunkPopulationLockId);
if($xx !== 0 || $zz !== 0){ //avoid registering it twice for the center chunk; we already did that above
$this->registerChunkLoader($temporaryChunkLoader, $x + $xx, $z + $zz);
}
}
}
$task = new PopulationTask($this, $x, $z, $chunk, $temporaryChunkLoader);
$task = new PopulationTask($this, $x, $z, $chunk, $temporaryChunkLoader, $chunkPopulationLockId);
$workerId = $this->workerPool->selectWorker();
if(!isset($this->generatorRegisteredWorkers[$workerId])){
$this->registerGeneratorToWorker($workerId);
@ -2843,10 +2873,10 @@ class World implements ChunkManager{
}
/**
* @param Chunk[] $adjacentChunks
* @param Chunk[] $adjacentChunks chunkHash => chunk
* @phpstan-param array<int, Chunk> $adjacentChunks
*/
public function generateChunkCallback(int $x, int $z, Chunk $chunk, array $adjacentChunks, ChunkLoader $temporaryChunkLoader) : void{
public function generateChunkCallback(ChunkLockId $chunkLockId, int $x, int $z, Chunk $chunk, array $adjacentChunks, ChunkLoader $temporaryChunkLoader) : void{
Timings::$generationCallback->startTiming();
for($xx = -1; $xx <= 1; ++$xx){
@ -2856,31 +2886,52 @@ class World implements ChunkManager{
}
if(isset($this->chunkPopulationRequestMap[$index = World::chunkHash($x, $z)]) && isset($this->activeChunkPopulationTasks[$index])){
$dirtyChunks = 0;
for($xx = -1; $xx <= 1; ++$xx){
for($zz = -1; $zz <= 1; ++$zz){
$this->unlockChunk($x + $xx, $z + $zz);
if(!$this->unlockChunk($x + $xx, $z + $zz, $chunkLockId)){
$dirtyChunks++;
}
}
}
if($dirtyChunks === 0){
$oldChunk = $this->loadChunk($x, $z);
$this->setChunk($x, $z, $chunk);
$oldChunk = $this->loadChunk($x, $z);
$this->setChunk($x, $z, $chunk);
foreach($adjacentChunks as $adjacentChunkHash => $adjacentChunk){
World::getXZ($adjacentChunkHash, $xAdjacentChunk, $zAdjacentChunk);
$this->setChunk($xAdjacentChunk, $zAdjacentChunk, $adjacentChunk);
}
if(($oldChunk === null or !$oldChunk->isPopulated()) and $chunk->isPopulated()){
(new ChunkPopulateEvent($this, $x, $z, $chunk))->call();
foreach($this->getChunkListeners($x, $z) as $listener){
$listener->onChunkPopulated($x, $z, $chunk);
foreach($adjacentChunks as $adjacentChunkHash => $adjacentChunk){
World::getXZ($adjacentChunkHash, $xAdjacentChunk, $zAdjacentChunk);
$this->setChunk($xAdjacentChunk, $zAdjacentChunk, $adjacentChunk);
}
if(($oldChunk === null or !$oldChunk->isPopulated()) and $chunk->isPopulated()){
(new ChunkPopulateEvent($this, $x, $z, $chunk))->call();
foreach($this->getChunkListeners($x, $z) as $listener){
$listener->onChunkPopulated($x, $z, $chunk);
}
}
}else{
$this->logger->debug("Discarding population result for chunk x=$x,z=$z - terrain was modified on the main thread before async population completed");
}
//This needs to be in this specific spot because user code might call back to orderChunkPopulation().
//If it does, and finds the promise, and doesn't find an active task associated with it, it will schedule
//another PopulationTask. We don't want that because we're here processing the results.
//We can't remove the promise from the array before setting the chunks in the world because that would lead
//to the same problem. Therefore, it's necessary that this code be split into two if/else, with this in the
//middle.
unset($this->activeChunkPopulationTasks[$index]);
$promise = $this->chunkPopulationRequestMap[$index];
unset($this->chunkPopulationRequestMap[$index]);
$promise->resolve($chunk);
if($dirtyChunks === 0){
$promise = $this->chunkPopulationRequestMap[$index];
unset($this->chunkPopulationRequestMap[$index]);
$promise->resolve($chunk);
}else{
//request failed, stick it back on the queue
//we didn't resolve the promise or touch it in any way, so any fake chunk loaders are still valid and
//don't need to be added a second time.
$this->chunkPopulationRequestQueue->enqueue($index);
}
$this->drainPopulationRequestQueue();
}

View File

@ -27,6 +27,7 @@ use pocketmine\data\bedrock\BiomeIds;
use pocketmine\scheduler\AsyncTask;
use pocketmine\utils\AssumptionFailedError;
use pocketmine\world\ChunkLoader;
use pocketmine\world\ChunkLockId;
use pocketmine\world\format\BiomeArray;
use pocketmine\world\format\Chunk;
use pocketmine\world\format\io\FastChunkSerializer;
@ -40,6 +41,7 @@ use function intdiv;
class PopulationTask extends AsyncTask{
private const TLS_KEY_WORLD = "world";
private const TLS_KEY_CHUNK_LOADER = "chunkLoader";
private const TLS_KEY_LOCK_ID = "chunkLockId";
/** @var int */
public $worldId;
@ -53,7 +55,7 @@ class PopulationTask extends AsyncTask{
private string $adjacentChunks;
public function __construct(World $world, int $chunkX, int $chunkZ, ?Chunk $chunk, ChunkLoader $temporaryChunkLoader){
public function __construct(World $world, int $chunkX, int $chunkZ, ?Chunk $chunk, ChunkLoader $temporaryChunkLoader, ChunkLockId $chunkLockId){
$this->worldId = $world->getId();
$this->chunkX = $chunkX;
$this->chunkZ = $chunkZ;
@ -66,6 +68,7 @@ class PopulationTask extends AsyncTask{
$this->storeLocal(self::TLS_KEY_WORLD, $world);
$this->storeLocal(self::TLS_KEY_CHUNK_LOADER, $temporaryChunkLoader);
$this->storeLocal(self::TLS_KEY_LOCK_ID, $chunkLockId);
}
public function onRun() : void{
@ -131,6 +134,8 @@ class PopulationTask extends AsyncTask{
$world = $this->fetchLocal(self::TLS_KEY_WORLD);
/** @var ChunkLoader $temporaryChunkLoader */
$temporaryChunkLoader = $this->fetchLocal(self::TLS_KEY_CHUNK_LOADER);
/** @var ChunkLockId $lockId */
$lockId = $this->fetchLocal(self::TLS_KEY_LOCK_ID);
if($world->isLoaded()){
$chunk = $this->chunk !== null ?
FastChunkSerializer::deserializeTerrain($this->chunk) :
@ -151,7 +156,7 @@ class PopulationTask extends AsyncTask{
}
}
$world->generateChunkCallback($this->chunkX, $this->chunkZ, $chunk, $adjacentChunks, $temporaryChunkLoader);
$world->generateChunkCallback($lockId, $this->chunkX, $this->chunkZ, $chunk, $adjacentChunks, $temporaryChunkLoader);
}
}
}