Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use coordinate-based locking to increase chunk system parallelism #74

Merged
merged 1 commit into from
May 15, 2023

Conversation

Spottedleaf
Copy link
Member

See patch notes for implementation details.

This patch should massively increase performance in Folia due to the significantly better locking behavior.

As with mainline Folia, builds are not provided you must build it yourself. If you already know how to build regular Folia, just use
your git client to checkout to the dev/locking branch (and please verify via current HEAD that you have the latest from the branch).

Please report any issues with this branch on this PR.

@Malfrador
Copy link
Member

I did a bit of testing with this branch and bots joining + spawnRadius = 50k. Chunk loading seems to be smoother, which is nice.

However, in two out of three tries the server completely locked up after about 10 minutes.
For some time before that /tps showed a different region at ~2 TPS every time I ran it. After some time, one region apparently completely died, showing up at 5000 mspt.
after this it was also no longer possible to join (even though I was in a different region).

jstack dump of the unresponsive server

@Spottedleaf
Copy link
Member Author

How many regions? You may have hit the scheduler's limit.

@Malfrador
Copy link
Member

javaw_vuotdg7t0t
Well, 5. Which is also seems off considering its 194 bots with spawnradius 50k.

@Spottedleaf
Copy link
Member Author

I have reverted the change to the region shift, please try again.

@Malfrador
Copy link
Member

Did three tests again, no locking up. Highest mspt is at 14ms. Region count also seems a lot more plausible now, and gen rate went up massively. I guess that revert helped.
javaw_MSzQjWHCyx

@ricsal1
Copy link

ricsal1 commented May 14, 2023

Got this on previous built, from 19h ago, might help.

log
[17:32:24] [Region Scheduler Thread #0/ERROR]: Too many chained neighbor updates. Skipping the rest. First skipped position: -3966, 42, 6380
[17:32:39] [Region Scheduler Thread #0/ERROR]: Too many chained neighbor updates. Skipping the rest. First skipped position: -3808, 53, 6448
[17:32:46] [Region Scheduler Thread #0/ERROR]: Too many chained neighbor updates. Skipping the rest. First skipped position: -3888, 62, 6448
[17:32:51] [Tuinity Chunk System Worker #3/ERROR]: [ChunkTaskScheduler] Chunk system error at chunk (-96,133), holder: NewChunkHolder{world=world, chunkX=-96, chunkZ=133, entityChunkFromDisk=false, lastChunkCompletion={chunk_class=net.minecraft.world.level.chunk.ProtoChunk,status=minecraft:liquid_carvers}, currentGenStatus=minecraft:liquid_carvers, requestedGenStatus=minecraft:full, generationTask=ChunkProgressionTask{class: io.papermc.paper.chunk.system.scheduling.ChunkUpgradeGenericStatusTask, for world: world, chunk: (-96,133), hashcode: 292180800, priority: COMPLETING, status: minecraft:features, scheduled: true}, generationTaskStatus=minecraft:features, priority=NORMAL, priorityLocked=false, neighbourRequestedPriority=NORMAL, effective_priority=NORMAL, oldTicketLevel=33, currentTicketLevel=33, totalNeighboursUsingThisChunk=9, fullNeighbourChunksLoadedBitset=3247203, chunkStatusRaw=0, currentChunkStatus=INACCESSIBLE, pendingChunkStatus=INACCESSIBLE, is_unload_safe=ticket_level, killed=false}, exception:
java.lang.Throwable: net.minecraft.ReportedException: Feature placement
	at io.papermc.paper.chunk.system.scheduling.ChunkTaskScheduler.unrecoverableChunkSystemFailure(ChunkTaskScheduler.java:280) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at io.papermc.paper.chunk.system.scheduling.NewChunkHolder.lambda$setGenerationTask$4(NewChunkHolder.java:1691) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at io.papermc.paper.chunk.system.scheduling.ChunkProgressionTask.complete0(ChunkProgressionTask.java:95) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at io.papermc.paper.chunk.system.scheduling.ChunkProgressionTask.complete(ChunkProgressionTask.java:75) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at io.papermc.paper.chunk.system.scheduling.ChunkUpgradeGenericStatusTask.run(ChunkUpgradeGenericStatusTask.java:105) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at io.papermc.paper.chunk.system.scheduling.queue.RadiusAwarePrioritisedExecutor$Task.run(RadiusAwarePrioritisedExecutor.java:468) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at ca.spottedleaf.concurrentutil.executor.standard.PrioritisedThreadedTaskQueue$PrioritisedTask.executeInternal(PrioritisedThreadedTaskQueue.java:351) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at ca.spottedleaf.concurrentutil.executor.standard.PrioritisedThreadedTaskQueue.executeTask(PrioritisedThreadedTaskQueue.java:118) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at ca.spottedleaf.concurrentutil.executor.standard.PrioritisedThreadPool$PrioritisedThread.pollTasks(PrioritisedThreadPool.java:274) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at ca.spottedleaf.concurrentutil.executor.standard.PrioritisedQueueExecutorThread.run(PrioritisedQueueExecutorThread.java:62) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
Caused by: net.minecraft.ReportedException: Feature placement
	at net.minecraft.world.level.chunk.ChunkGenerator.addVanillaDecorations(ChunkGenerator.java:463) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ChunkGenerator.applyBiomeDecoration(ChunkGenerator.java:475) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ChunkGenerator.applyBiomeDecoration(ChunkGenerator.java:470) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ChunkStatus.lambda$static$12(ChunkStatus.java:173) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ChunkStatus.generate(ChunkStatus.java:305) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at io.papermc.paper.chunk.system.scheduling.ChunkUpgradeGenericStatusTask.run(ChunkUpgradeGenericStatusTask.java:86) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	... 5 more
Caused by: java.lang.NullPointerException: Cannot read field "capturedTileEntities" because the return value of "net.minecraft.server.level.ServerLevel.getCurrentWorldData()" is null
	at net.minecraft.world.level.chunk.LevelChunk.getBlockEntity(LevelChunk.java:565) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.LevelChunk.getBlockEntity(LevelChunk.java:558) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ImposterProtoChunk.getBlockEntity(ImposterProtoChunk.java:85) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.server.level.WorldGenRegion.getBlockEntity(WorldGenRegion.java:253) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.block.entity.RandomizableContainerBlockEntity.setLootTable(RandomizableContainerBlockEntity.java:39) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.levelgen.feature.MonsterRoomFeature.place(MonsterRoomFeature.java:111) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.levelgen.feature.Feature.place(Feature.java:155) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.levelgen.feature.ConfiguredFeature.place(ConfiguredFeature.java:25) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.levelgen.placement.PlacedFeature.lambda$placeWithContext$4(PlacedFeature.java:52) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183) ~[?:?]
	at java.util.stream.Streams$StreamBuilderImpl.forEachRemaining(Streams.java:411) ~[?:?]
	at java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:762) ~[?:?]
	at java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276) ~[?:?]
	at java.util.stream.Streams$StreamBuilderImpl.forEachRemaining(Streams.java:411) ~[?:?]
	at java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:762) ~[?:?]
	at java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276) ~[?:?]
	at java.util.stream.Streams$StreamBuilderImpl.forEachRemaining(Streams.java:411) ~[?:?]
	at java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:762) ~[?:?]
	at java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183) ~[?:?]
	at java.util.stream.IntPipeline$1$1.accept(IntPipeline.java:180) ~[?:?]
	at java.util.stream.Streams$RangeIntSpliterator.forEachRemaining(Streams.java:104) ~[?:?]
	at java.util.Spliterator$OfInt.forEachRemaining(Spliterator.java:711) ~[?:?]
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) ~[?:?]
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) ~[?:?]
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?]
	at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596) ~[?:?]
	at java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276) ~[?:?]
	at java.util.stream.Streams$StreamBuilderImpl.forEachRemaining(Streams.java:411) ~[?:?]
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) ~[?:?]
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) ~[?:?]
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?]
	at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596) ~[?:?]
	at net.minecraft.world.level.levelgen.placement.PlacedFeature.placeWithContext(PlacedFeature.java:51) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.levelgen.placement.PlacedFeature.placeWithBiomeCheck(PlacedFeature.java:37) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ChunkGenerator.addVanillaDecorations(ChunkGenerator.java:445) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ChunkGenerator.applyBiomeDecoration(ChunkGenerator.java:475) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ChunkGenerator.applyBiomeDecoration(ChunkGenerator.java:470) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ChunkStatus.lambda$static$12(ChunkStatus.java:173) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ChunkStatus.generate(ChunkStatus.java:305) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at io.papermc.paper.chunk.system.scheduling.ChunkUpgradeGenericStatusTask.run(ChunkUpgradeGenericStatusTask.java:86) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	... 5 more
[17:32:54] [Region Scheduler Thread #0/ERROR]: Too many chained neighbor updates. Skipping the rest. First skipped position: -3984, 45, 6289
[17:32:54] [Region Scheduler Thread #0/ERROR]: [io.papermc.paper.threadedregions.TickRegionScheduler] Region #25 centered at chunk [-247, 142] in world 'world' failed to tick:
java.lang.RuntimeException: Chunk system crash propagated from unrecoverableChunkSystemFailure
	at io.papermc.paper.chunk.system.scheduling.ChunkTaskScheduler.lambda$unrecoverableChunkSystemFailure$1(ChunkTaskScheduler.java:303) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at io.papermc.paper.threadedregions.RegionizedTaskQueue$PrioritisedQueue$ChunkBasedPriorityTask.executeInternal(RegionizedTaskQueue.java:511) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at io.papermc.paper.threadedregions.RegionizedTaskQueue$PrioritisedQueue.executeTask(RegionizedTaskQueue.java:441) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at io.papermc.paper.threadedregions.RegionizedTaskQueue$RegionTaskQueueData.drainTasks(RegionizedTaskQueue.java:271) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.server.MinecraftServer.tickServer(MinecraftServer.java:1523) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at io.papermc.paper.threadedregions.TickRegions$ConcreteRegionTickHandle.tickRegion(TickRegions.java:360) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at io.papermc.paper.threadedregions.TickRegionScheduler$RegionScheduleHandle.runTick(TickRegionScheduler.java:385) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at ca.spottedleaf.concurrentutil.scheduler.SchedulerThreadPool$TickThreadRunner.run(SchedulerThreadPool.java:525) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at java.lang.Thread.run(Thread.java:833) ~[?:?]
Caused by: net.minecraft.ReportedException: Feature placement
	at net.minecraft.world.level.chunk.ChunkGenerator.addVanillaDecorations(ChunkGenerator.java:463) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ChunkGenerator.applyBiomeDecoration(ChunkGenerator.java:475) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ChunkGenerator.applyBiomeDecoration(ChunkGenerator.java:470) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ChunkStatus.lambda$static$12(ChunkStatus.java:173) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ChunkStatus.generate(ChunkStatus.java:305) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at io.papermc.paper.chunk.system.scheduling.ChunkUpgradeGenericStatusTask.run(ChunkUpgradeGenericStatusTask.java:86) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at io.papermc.paper.chunk.system.scheduling.queue.RadiusAwarePrioritisedExecutor$Task.run(RadiusAwarePrioritisedExecutor.java:468) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at ca.spottedleaf.concurrentutil.executor.standard.PrioritisedThreadedTaskQueue$PrioritisedTask.executeInternal(PrioritisedThreadedTaskQueue.java:351) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at ca.spottedleaf.concurrentutil.executor.standard.PrioritisedThreadedTaskQueue.executeTask(PrioritisedThreadedTaskQueue.java:118) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at ca.spottedleaf.concurrentutil.executor.standard.PrioritisedThreadPool$PrioritisedThread.pollTasks(PrioritisedThreadPool.java:274) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at ca.spottedleaf.concurrentutil.executor.standard.PrioritisedQueueExecutorThread.run(PrioritisedQueueExecutorThread.java:62) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
Caused by: java.lang.NullPointerException: Cannot read field "capturedTileEntities" because the return value of "net.minecraft.server.level.ServerLevel.getCurrentWorldData()" is null
	at net.minecraft.world.level.chunk.LevelChunk.getBlockEntity(LevelChunk.java:565) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.LevelChunk.getBlockEntity(LevelChunk.java:558) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ImposterProtoChunk.getBlockEntity(ImposterProtoChunk.java:85) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.server.level.WorldGenRegion.getBlockEntity(WorldGenRegion.java:253) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.block.entity.RandomizableContainerBlockEntity.setLootTable(RandomizableContainerBlockEntity.java:39) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.levelgen.feature.MonsterRoomFeature.place(MonsterRoomFeature.java:111) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.levelgen.feature.Feature.place(Feature.java:155) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.levelgen.feature.ConfiguredFeature.place(ConfiguredFeature.java:25) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.levelgen.placement.PlacedFeature.lambda$placeWithContext$4(PlacedFeature.java:52) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183) ~[?:?]
	at java.util.stream.Streams$StreamBuilderImpl.forEachRemaining(Streams.java:411) ~[?:?]
	at java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:762) ~[?:?]
	at java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276) ~[?:?]
	at java.util.stream.Streams$StreamBuilderImpl.forEachRemaining(Streams.java:411) ~[?:?]
	at java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:762) ~[?:?]
	at java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276) ~[?:?]
	at java.util.stream.Streams$StreamBuilderImpl.forEachRemaining(Streams.java:411) ~[?:?]
	at java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:762) ~[?:?]
	at java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183) ~[?:?]
	at java.util.stream.IntPipeline$1$1.accept(IntPipeline.java:180) ~[?:?]
	at java.util.stream.Streams$RangeIntSpliterator.forEachRemaining(Streams.java:104) ~[?:?]
	at java.util.Spliterator$OfInt.forEachRemaining(Spliterator.java:711) ~[?:?]
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) ~[?:?]
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) ~[?:?]
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?]
	at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596) ~[?:?]
	at java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276) ~[?:?]
	at java.util.stream.Streams$StreamBuilderImpl.forEachRemaining(Streams.java:411) ~[?:?]
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) ~[?:?]
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) ~[?:?]
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?]
	at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596) ~[?:?]
	at net.minecraft.world.level.levelgen.placement.PlacedFeature.placeWithContext(PlacedFeature.java:51) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.levelgen.placement.PlacedFeature.placeWithBiomeCheck(PlacedFeature.java:37) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ChunkGenerator.addVanillaDecorations(ChunkGenerator.java:445) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ChunkGenerator.applyBiomeDecoration(ChunkGenerator.java:475) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ChunkGenerator.applyBiomeDecoration(ChunkGenerator.java:470) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ChunkStatus.lambda$static$12(ChunkStatus.java:173) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at net.minecraft.world.level.chunk.ChunkStatus.generate(ChunkStatus.java:305) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at io.papermc.paper.chunk.system.scheduling.ChunkUpgradeGenericStatusTask.run(ChunkUpgradeGenericStatusTask.java:86) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at io.papermc.paper.chunk.system.scheduling.queue.RadiusAwarePrioritisedExecutor$Task.run(RadiusAwarePrioritisedExecutor.java:468) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at ca.spottedleaf.concurrentutil.executor.standard.PrioritisedThreadedTaskQueue$PrioritisedTask.executeInternal(PrioritisedThreadedTaskQueue.java:351) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at ca.spottedleaf.concurrentutil.executor.standard.PrioritisedThreadedTaskQueue.executeTask(PrioritisedThreadedTaskQueue.java:118) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at ca.spottedleaf.concurrentutil.executor.standard.PrioritisedThreadPool$PrioritisedThread.pollTasks(PrioritisedThreadPool.java:274) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
	at ca.spottedleaf.concurrentutil.executor.standard.PrioritisedQueueExecutorThread.run(PrioritisedQueueExecutorThread.java:62) ~[folia-1.19.4.jar:git-Folia-"4c11d7e"]
[17:32:54] [Region shutdown thread/INFO]: [RegionShutdownThread] Awaiting scheduler termination for 60s
[17:32:54] [Region shutdown thread/INFO]: [RegionShutdownThread] Scheduler halted
[17:32:54] [Region shutdown thread/INFO]: Stopping server

@Spottedleaf
Copy link
Member Author

Got this on previous built, from 19h ago, might help.
log

I can only see this happening if you have a plugin used the regenerate chunk API. Is this the case?

@ricsal1
Copy link

ricsal1 commented May 14, 2023

I can only see this happening if you have a plugin used the regenerate chunk API. Is this the case?

Hi Leaf, I don't have any plugins generating chunks.
This test ran with minecraft-stress-test connected, simulating players

BTW on current built I got these controlled errores:
"[20:30:48 ERROR]: Detected setBlock in a far chunk [-126, 408], pos: BlockPos{x=-2013, y=31, z=6540}, status: minecraft:features, currently generating: ResourceKey[minecraft:worldgen/placed_feature / minecraft:large_dripstone]"

"[21:14:34 ERROR]: Failed to fetch mob spawner entity at (-3540, 21, -529)"

@Spottedleaf
Copy link
Member Author

I can only see this happening if you have a plugin used the regenerate chunk API. Is this the case?

Hi Leaf, I don't have any plugins generating chunks. This test ran with minecraft-stress-test connected, simulating players

BTW on current built I got these controlled errores: "[20:30:48 ERROR]: Detected setBlock in a far chunk [-126, 408], pos: BlockPos{x=-2013, y=31, z=6540}, status: minecraft:features, currently generating: ResourceKey[minecraft:worldgen/placed_feature / minecraft:large_dripstone]"

"[21:14:34 ERROR]: Failed to fetch mob spawner entity at (-3540, 21, -529)"

I have reviewed the stacktrace again only to find that the line number on ImposterProtoChunk does not match what it should.
It looks like the diff I inserted there to prevent this crash is not on your Folia build, even though the commit # indicates it should be. Can you re-run applyPatches, and make a new build and test again?

A significant overhead in Folia comes from the chunk system's
locks, the ticket lock and the scheduling lock. The public
test server, which had ~330 players, had signficant performance
problems with these locks: ~80% of the time spent ticking
was _waiting_ for the locks to free. Given that it used
around 15 cores total at peak, this is a complete and utter loss
of potential.

To address this issue, I have replaced the ticket lock and scheduling
lock with two ReentrantAreaLocks. The ReentrantAreaLock takes a
shift, which is used internally to group positions into sections.
This grouping is neccessary, as the possible radius of area that
needs to be acquired for any given lock usage is up to 64. As such,
the shift is critical to reduce the number of areas required to lock
for any lock operation. Currently, it is set to a shift of 6, which
is identical to the ticket level propagation shift (and, it must be
at least the ticket level propagation shift AND the region shift).

The chunk system locking changes required a complete rewrite of the
chunk system tick, chunk system unload, and chunk system ticket level
propagation - as all of the previous logic only works with a single
global lock.

This does introduce two other section shifts: the lock shift, and the
ticket shift. The lock shift is simply what shift the area locks use,
and the ticket shift represents the size of the ticket sections.
Currently, these values are just set to the region shift for simplicity.
However, they are not arbitrary: the lock shift must be at least the size
of the ticket shift and must be at least the size of the region shift.
The ticket shift must also be >= the ceil(log2(max ticket level source)).

The chunk system's ticket propagator is now global state, instead of
region state. This cleans up the logic for ticket levels significantly,
and removes usage of the region lock in this area, but it also means
that the addition of a ticket no longer creates a region. To alleviate
the side effects of this change, the global tick thread now processes
ticket level updates for each world every tick to guarantee eventual
ticket level processing. The chunk system also provides a hook to
process ticket level changes in a given _section_, so that the
region queue can guarantee that after adding its reference counter
that the region section is created/exists/wont be destroyed.

The ticket propagator operates by updating the sources in a single ticket
section, and propagating the updates to its 1 radius neighbours. This
allows the ticket updates to occur in parallel or selectively (see above).
Currently, the process ticket level update function operates by
polling from a concurrent queue of sections to update and simply
invoking the single section update logic. This allows the function
to operate completely in parallel, provided the queue is ordered right.
Additionally, this limits the area used in the ticket/scheduling lock
when processing updates, which should massively increase parallelism compared
to before.

The chunk system ticket addition for expirable ticket types has been modified
to no longer track exact tick deadlines, as this relies on what region the
ticket is in. Instead, the chunk system tracks a map of
lock section -> (chunk coordinate -> expire ticket count) and every ticket
has been changed to have a removeDelay count that is decremented each tick.
Each region searches its own sections to find tickets to try to expire.

Chunk system unloading has been modified to track unloads by lock section.
The ordering is determined by which section a chunk resides in.
The unload process now removes from unload sections and processes
the full unload stages (1, 2, 3) before moving to the next section, if possible.
This allows the unload logic to only hold one lock section at a time for
each lock, which is a massive parallelism increase.

In stress testing, these changes lowered the locking overhead to only 5%
from ~70%, which completely fix the original problem as described.
@Spottedleaf Spottedleaf changed the title [DEV-BRANCH] Use coordinate-based locking to increase chunk system parallelism Use coordinate-based locking to increase chunk system parallelism May 15, 2023
@Spottedleaf Spottedleaf marked this pull request as ready for review May 15, 2023 02:45
@Spottedleaf
Copy link
Member Author

This branch seems stable enough for Folia, so I'll merge it now.

@Spottedleaf Spottedleaf merged commit 31b5b15 into master May 15, 2023
@Spottedleaf Spottedleaf deleted the dev/locking branch May 15, 2023 02:46
@ricsal1
Copy link

ricsal1 commented May 15, 2023

I have reviewed the stacktrace again only to find that the line number on ImposterProtoChunk does not match what it should. It looks like the diff I inserted there to prevent this crash is not on your Folia build, even though the commit # indicates it should be. Can you re-run applyPatches, and make a new build and test again?

Might have missed a step in the built, anyway the last built from this branch did not produce any problems besides the one I last posted, all controlled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants