Skip to content
This repository was archived by the owner on Dec 29, 2023. It is now read-only.

GH-45 Backport Paper 1.13.2 Async Chunk Generation (and Loading) (Resolves #45) #86

Merged
merged 18 commits into from
Oct 21, 2022

Conversation

P3ridot
Copy link
Member

@P3ridot P3ridot commented Oct 14, 2022

#45

Finally after lots of tries I achieved Async Chunk Generation (1 thread per world) by backporting Paper 1.13.2 Async Chunk System (thanks for idea @LewUwU). When I will be sure everything works fine I will merge this to dev.

Titanium with this patch can be obtained from Jenkins

@P3ridot P3ridot added the enhancement Enhancement to existing thing label Oct 14, 2022
@P3ridot P3ridot changed the base branch from main to dev October 14, 2022 10:13
@P3ridot P3ridot changed the title GH-45 Backport Paper 1.13.2 Async Chunk Generation (and Loading) (#45) GH-45 Backport Paper 1.13.2 Async Chunk Generation (and Loading) (Resolves #45) Oct 14, 2022
@ghost
Copy link

ghost commented Oct 16, 2022

After testing a bit in production, everything seems to work fine

@ghost
Copy link

ghost commented Oct 16, 2022

@P3ridot

[14:07:36] [Server thread/ERROR]: Encountered an unexpected exception
java.lang.RuntimeException: Already decorating
	at net.minecraft.server.v1_8_R3.BiomeDecorator.a(SourceFile:25)
	at net.minecraft.server.v1_8_R3.BiomeBase.a(BiomeBase.java:343)
	at net.minecraft.server.v1_8_R3.ChunkProviderGenerate.getChunkAt(ChunkProviderGenerate.java:357)
	at org.bukkit.craftbukkit.v1_8_R3.generator.NormalChunkGenerator.getChunkAt(NormalChunkGenerator.java:51)
	at net.minecraft.server.v1_8_R3.ChunkProviderServer.getChunkAt(ChunkProviderServer.java:431)
	at net.minecraft.server.v1_8_R3.Chunk.loadNearby(Chunk.java:1376)
	at net.minecraft.server.v1_8_R3.ChunkProviderServer.postChunk(ChunkProviderServer.java:337)
	at net.minecraft.server.v1_8_R3.PaperAsyncChunkProvider$PendingChunk.postChunk(PaperAsyncChunkProvider.java:513)
	at net.minecraft.server.v1_8_R3.PaperAsyncChunkProvider.processMainThreadQueue(PaperAsyncChunkProvider.java:98)
	at net.minecraft.server.v1_8_R3.PaperAsyncChunkProvider.processMainThreadQueue(PaperAsyncChunkProvider.java:91)
	at net.minecraft.server.v1_8_R3.PaperAsyncChunkProvider.processMainThreadQueue(PaperAsyncChunkProvider.java:86)
	at net.minecraft.server.v1_8_R3.PaperAsyncChunkProvider.processMainThreadQueue(PaperAsyncChunkProvider.java:79)
	at net.minecraft.server.v1_8_R3.MinecraftServer.B(MinecraftServer.java:1074)
	at net.minecraft.server.v1_8_R3.DedicatedServer.B(DedicatedServer.java:308)
	at net.minecraft.server.v1_8_R3.MinecraftServer.A(MinecraftServer.java:1040)
	at net.minecraft.server.v1_8_R3.MinecraftServer.run(MinecraftServer.java:852)
	at net.minecraft.server.v1_8_R3.MinecraftServer.lambda$spin$0(MinecraftServer.java:134)
	at java.lang.Thread.run(Thread.java:829) ~[?:?]

Can be reproduced some times by creating a world with Multiverse-Core (/mv create worldname normal)

@P3ridot
Copy link
Member Author

P3ridot commented Oct 16, 2022

This patch should fix this - that exception just can't appear in any other situation. Also from my small investigation Multiverse-Core don't do anything with BiomeDecorator or something.

@P3ridot
Copy link
Member Author

P3ridot commented Oct 17, 2022

This patch should fix this - that exception just can't appear in any other situation. Also from my small investigation Multiverse-Core don't do anything with BiomeDecorator or something.

So @LewUwU are you using Titanium or just this patch? If second option check the result when you use patch mentioned above

@ghost
Copy link

ghost commented Oct 18, 2022

This patch should fix this - that exception just can't appear in any other situation. Also from my small investigation Multiverse-Core don't do anything with BiomeDecorator or something.

So @LewUwU are you using Titanium or just this patch? If second option check the result when you use patch mentioned above

Just this patch, i fixed it by allowing concurrently access to the BiomeDecorator but your option should fix some unknown bugs

@P3ridot
Copy link
Member Author

P3ridot commented Oct 18, 2022

This patch should fix this - that exception just can't appear in any other situation. Also from my small investigation Multiverse-Core don't do anything with BiomeDecorator or something.

So @LewUwU are you using Titanium or just this patch? If second option check the result when you use patch mentioned above

Just this patch, i fixed it by allowing concurrently access to the BiomeDecorator but your option should fix some unknown bugs

Okay, so if everything will work I will merge this PR in a few days

@ghost
Copy link

ghost commented Oct 18, 2022

After viewing some Spark profilers with this patch, there's only lag spikes in Chunk#loadNearby (up to 1k ms), i will see what can be done to optimise that method

@P3ridot
Copy link
Member Author

P3ridot commented Oct 18, 2022

After viewing some Spark profilers with this patch, there's only lag spikes in Chunk#loadNearby (up to 1k ms), i will see what can be done to optimise that method

After looking a little in code there is no much calculations - probably chunks are loaded synchronously, but I have to make sure

@ghost
Copy link

ghost commented Oct 18, 2022

After viewing some Spark profilers with this patch, there's only lag spikes in Chunk#loadNearby (up to 1k ms), i will see what can be done to optimise that method

After looking a little in code there is no much calculations - probably chunks are loaded synchronously, but I have to make sure

AFAIK there's chunk population in that method (Also we should change the isChunkLoaded to getChunkIfLoaded)

@P3ridot
Copy link
Member Author

P3ridot commented Oct 18, 2022

AFAIK there's chunk population in that method (Also we should change the isChunkLoaded to getChunkIfLoaded)

Oh, yeah. And why getChunkIfLoaded? I don't get the reason why 🤔

@ghost
Copy link

ghost commented Oct 18, 2022

AFAIK there's chunk population in that method (Also we should change the isChunkLoaded to getChunkIfLoaded)

Oh, yeah. And why getChunkIfLoaded? I don't get the reason why 🤔

isChunkLoaded -> getChunkAt can be replaced by getChunkIfLoaded

@P3ridot
Copy link
Member Author

P3ridot commented Oct 19, 2022

AFAIK there's chunk population in that method (Also we should change the isChunkLoaded to getChunkIfLoaded)

Oh, yeah. And why getChunkIfLoaded? I don't get the reason why 🤔

isChunkLoaded -> getChunkAt can be replaced by getChunkIfLoaded

Right, I did that and everything looks fine - but despite that it's still have to be optimized even more

@P3ridot
Copy link
Member Author

P3ridot commented Oct 19, 2022

I have no idea how we could optimise this even more - we would probably have to optimise decorating (so generating trees etc.) or find a way to do that asynchronously

@ghost
Copy link

ghost commented Oct 19, 2022

I have no idea how we could optimise this even more - we would probably have to optimise decorating (so generating trees etc.) or find a way to do that asynchronously

The generators should be recoded to support async gen too, some biomes decoration got buggy after being generated (water lily in the air and etc)

@P3ridot
Copy link
Member Author

P3ridot commented Oct 20, 2022

I have no idea how we could optimise this even more - we would probably have to optimise decorating (so generating trees etc.) or find a way to do that asynchronously

The generators should be recoded to support async gen too, some biomes decoration got buggy after being generated (water lily in the air and etc)

I know it should, but it wouldn't be easy. However, after looking deeper in the spark, it looks like sometimes even 50% are used by ore generation. So maybe there, I'll try to optimize that.

Oh and after testing I can't reproduce that flying lilypads 🤔

@P3ridot
Copy link
Member Author

P3ridot commented Oct 20, 2022

I'll try to add some micro optimizations, but currently I don't have any more ideas how to optimize chunks decoration. So I'll merge this PR and optimize terrain decoration in the future.

Currently we probably didn't save much performance, but maybe a little by offloading terrain generation to async threads...

At the beginning I have to revert this patch (this part with dirty blocks) - where I replaced array with map from fastutil...

Later I have to think about a way to set blocks in bulk (to do some calls once, not every block set)

@P3ridot
Copy link
Member Author

P3ridot commented Oct 20, 2022

So I optimized chunk decoration (probably only a little, but it's still something). Now we have to pray that everything still works (mostly ore generation).

And like I said, I will merge this branch soon. We actually made terrain generation more efficient - before the server couldn't have stable 20 TPS when terrain generation was done synchronously.

Optimizing chunk decoration would be great, but unfortunately I don't have much time to think how to actually do that. In best scenario I would somehow do that asynchronously, but we have to think realistically.

@P3ridot P3ridot merged commit 6be0a2c into dev Oct 21, 2022
@P3ridot P3ridot deleted the async-chunks-gen branch October 21, 2022 08:10
@P3ridot
Copy link
Member Author

P3ridot commented Oct 21, 2022

To the moon! 🌕✨

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Enhancement to existing thing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant