Skip to content

Make map generation 12% faster. - #3377

Open
calebstenberg wants to merge 8 commits into
PixelGuys:masterfrom
calebstenberg:map-gen
Open

Make map generation 12% faster.#3377
calebstenberg wants to merge 8 commits into
PixelGuys:masterfrom
calebstenberg:map-gen

Conversation

@calebstenberg

@calebstenberg calebstenberg commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

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.

generate used to take 28% of CPU time and now takes 15.8% according to AMD uProf

Edit:
Now only down to 130 seconds. And uses 16.89% of CPU time

Edit 2:
Uses only 14.5% of CPU time

@calebstenberg
calebstenberg marked this pull request as draft July 17, 2026 02:16
@calebstenberg

Copy link
Copy Markdown
Contributor Author

The problem is that the margin doesn't work any more. I will try a couple things

@calebstenberg calebstenberg changed the title Make map generation 25% faster. Make map generation 12% faster. Jul 17, 2026
@calebstenberg

Copy link
Copy Markdown
Contributor Author

The problem is that the margin doesn't work any more. I will try a couple things

Fixed it

@calebstenberg
calebstenberg marked this pull request as ready for review July 17, 2026 15:20
@Wunka Wunka moved this to Easy to Review in PRs to review Jul 17, 2026

@IntegratedQuantum IntegratedQuantum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really interesting. I didn't know the random sampling was so influential.

Comment thread src/server/terrain/cavebiomegen/RandomBiomeDistribution.zig
Comment thread src/server/terrain/cave_layers.zig Outdated
@IntegratedQuantum IntegratedQuantum moved this from Easy to Review to In review in PRs to review Jul 17, 2026
@calebstenberg

calebstenberg commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Even with a faster layer search it still takes 17% of cpu time, but it's still better than 28%

@calebstenberg

Copy link
Copy Markdown
Contributor Author

Now it is only 14.5% of CPU time!

@calebstenberg

Copy link
Copy Markdown
Contributor Author

This effect is even more pronounced in layers with few biomes like the sky or void.

Comment thread src/server/terrain/cave_layers.zig Outdated
Comment thread src/server/terrain/cave_layers.zig Outdated
@@ -66,9 +68,32 @@ pub const CaveLayer = struct {
}

result.biomes = .init(main.worldArena, main.worldArena.dupe(*const Biome, biomes.items));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The biome list should be determined in registerCaveLayers before you subdivide the layer.
Then you don't need to clone and avoid needless allocations.

Comment thread src/server/terrain/cave_layers.zig Outdated
Comment thread src/server/terrain/cave_layers.zig Outdated
Comment thread src/server/terrain/cave_layers.zig Outdated
Comment thread src/server/terrain/cavebiomegen/RandomBiomeDistribution.zig Outdated
Comment thread src/server/terrain/cavebiomegen/RandomBiomeDistribution.zig Outdated
Comment thread src/server/terrain/cave_layers.zig
Comment thread src/server/terrain/cave_layers.zig Outdated
Comment thread src/server/terrain/cave_layers.zig Outdated
Comment thread src/server/terrain/cave_layers.zig Outdated
Comment thread src/server/terrain/cave_layers.zig Outdated
Comment thread src/server/terrain/cave_layers.zig Outdated
@calebstenberg

Copy link
Copy Markdown
Contributor Author

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 IntegratedQuantum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/server/terrain/cave_layers.zig Outdated
Comment thread src/server/terrain/cave_layers.zig Outdated
Comment thread src/server/terrain/cave_layers.zig Outdated
Comment thread src/server/terrain/cave_layers.zig Outdated
Comment thread src/server/terrain/cave_layers.zig Outdated
@calebstenberg

Copy link
Copy Markdown
Contributor Author

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?
I also can't think of a way to have the layers initially and then rebuild them with a bigger list without just wasting an allocation

@IntegratedQuantum

Copy link
Copy Markdown
Member

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.

Comment thread src/server/terrain/cave_layers.zig Outdated
var newLayers: main.List(CaveLayer) = .empty;

for (caveLayers.items) |layer| {
const splitLayers = splitLayer(main.worldArena, layer);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. What I propose is just this:

for (caveLayers.items) |layer| {
	splitLayer(main.worldArena, &newLayers, layer);
}

Comment thread src/server/terrain/cave_layers.zig Outdated
return null;
}

result.biomes = .init(main.worldArena, main.worldArena.dupe(*const Biome, biomes.items));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Comment thread src/server/terrain/cave_layers.zig Outdated
var newLayers: main.List(CaveLayer) = .empty;

for (caveLayers.items) |layer| {
const splitLayers = splitLayer(main.worldArena, layer);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. What I propose is just this:

for (caveLayers.items) |layer| {
	splitLayer(main.worldArena, &newLayers, layer);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

3 participants