Make map generation 12% faster. - #3377
Conversation
|
The problem is that the margin doesn't work any more. I will try a couple things |
Fixed it |
IntegratedQuantum
left a comment
There was a problem hiding this comment.
This is really interesting. I didn't know the random sampling was so influential.
|
Even with a faster layer search it still takes 17% of cpu time, but it's still better than 28% |
|
Now it is only 14.5% of CPU time! |
|
This effect is even more pronounced in layers with few biomes like the sky or void. |
| @@ -66,9 +68,32 @@ pub const CaveLayer = struct { | |||
| } | |||
|
|
|||
| result.biomes = .init(main.worldArena, main.worldArena.dupe(*const Biome, biomes.items)); | |||
There was a problem hiding this comment.
The biome list should be determined in registerCaveLayers before you subdivide the layer.
Then you don't need to clone and avoid needless allocations.
|
I'm on a business trip and won't be working on this for about a week just fyi |
Removed debug logging for biome details in cave layers.
IntegratedQuantum
left a comment
There was a problem hiding this comment.
Also https://github.com/PixelGuys/Cubyz/pull/3377/changes#r3607941472 still applies
How? Do you want it to not call splitLayers at all? If so then where? asset.zig where registerLayers is called? |
|
No I just want you to move the code that determines the biomes of each layer into the splitLayers function so you don't need to allocate pointless copies of the biome lists into the worldArena. |
| var newLayers: main.List(CaveLayer) = .empty; | ||
|
|
||
| for (caveLayers.items) |layer| { | ||
| const splitLayers = splitLayer(main.worldArena, layer); |
There was a problem hiding this comment.
Please don't store this temporary list in the worldArena.
Maybe it would be easiest to just pass a pointer to newLayers, this would avoid the need for a temporary list here.
There was a problem hiding this comment.
Problem is I need world alloc biomes and world alloc layers. Would you like me to pass both a pointer and an allocator? Then I can allocate biomes and update a single list.
There was a problem hiding this comment.
Yes. What I propose is just this:
for (caveLayers.items) |layer| {
splitLayer(main.worldArena, &newLayers, layer);
}
| return null; | ||
| } | ||
|
|
||
| result.biomes = .init(main.worldArena, main.worldArena.dupe(*const Biome, biomes.items)); |
There was a problem hiding this comment.
Why is this back here again?
What I mean in #3377 (comment) was only moving the checks, not the biome list generation. (That's why the code I gave you doesn't use the biome list)
| var newLayers: main.List(CaveLayer) = .empty; | ||
|
|
||
| for (caveLayers.items) |layer| { | ||
| const splitLayers = splitLayer(main.worldArena, layer); |
There was a problem hiding this comment.
Yes. What I propose is just this:
for (caveLayers.items) |layer| {
splitLayer(main.worldArena, &newLayers, layer);
}
Instead of guessing around until we find a biome in the layer, just store what biomes are in the layer as well and use that to sample.
This brought generating a new world seed 101 at max render from 142 sec to 125 sec.
generateused to take 28% of CPU time and now takes 15.8% according to AMD uProfEdit:
Now only down to 130 seconds. And uses 16.89% of CPU time
Edit 2:
Uses only 14.5% of CPU time