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.

Sign up for GitHub

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

Improve BiomeMask performance #3082

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Improve BiomeMask performance #3082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f73c4de
Improve BiomeMask performance
SirYwell Jan 15, 2025
3b4aeb3
comments
SirYwell Jan 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions ...ava/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_20_R2/PaperweightPlatformAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,11 +652,12 @@ public static void clearCounts(final LevelChunkSection section) throws IllegalAc

public static BiomeType adapt(Holder<Biome> biome, LevelAccessor levelAccessor) {
final Registry<Biome> biomeRegistry = levelAccessor.registryAccess().registryOrThrow(BIOME);
if (biomeRegistry.getKey(biome.value()) == null) {
return biomeRegistry.asHolderIdMap().getId(biome) == -1 ? BiomeTypes.OCEAN
: null;
final int id = biomeRegistry.getId(biome.value());
if (id < 0) {
// this shouldn't be the case, but other plugins can be weird
return BiomeTypes.OCEAN;
}
return BiomeTypes.get(biome.unwrapKey().orElseThrow().location().toString());
return BiomeTypes.getLegacy(id);
}

static void removeBeacon(BlockEntity beacon, LevelChunk levelChunk) {
Expand Down
9 changes: 5 additions & 4 deletions ...ava/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_20_R3/PaperweightPlatformAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,11 +652,12 @@ public static void clearCounts(final LevelChunkSection section) throws IllegalAc

public static BiomeType adapt(Holder<Biome> biome, LevelAccessor levelAccessor) {
final Registry<Biome> biomeRegistry = levelAccessor.registryAccess().registryOrThrow(BIOME);
if (biomeRegistry.getKey(biome.value()) == null) {
return biomeRegistry.asHolderIdMap().getId(biome) == -1 ? BiomeTypes.OCEAN
: null;
final int id = biomeRegistry.getId(biome.value());
if (id < 0) {
// this shouldn't be the case, but other plugins can be weird
return BiomeTypes.OCEAN;
}
return BiomeTypes.get(biome.unwrapKey().orElseThrow().location().toString());
return BiomeTypes.getLegacy(id);
}

static void removeBeacon(BlockEntity beacon, LevelChunk levelChunk) {
Expand Down
9 changes: 5 additions & 4 deletions ...ava/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_20_R4/PaperweightPlatformAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -645,11 +645,12 @@ public static void clearCounts(final LevelChunkSection section) throws IllegalAc

public static BiomeType adapt(Holder<Biome> biome, LevelAccessor levelAccessor) {
final Registry<Biome> biomeRegistry = levelAccessor.registryAccess().registryOrThrow(BIOME);
if (biomeRegistry.getKey(biome.value()) == null) {
return biomeRegistry.asHolderIdMap().getId(biome) == -1 ? BiomeTypes.OCEAN
: null;
final int id = biomeRegistry.getId(biome.value());
if (id < 0) {
// this shouldn't be the case, but other plugins can be weird
return BiomeTypes.OCEAN;
}
return BiomeTypes.get(biome.unwrapKey().orElseThrow().location().toString());
return BiomeTypes.getLegacy(id);
}

static void removeBeacon(BlockEntity beacon, LevelChunk levelChunk) {
Expand Down
9 changes: 5 additions & 4 deletions ...ava/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_R1/PaperweightPlatformAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,11 +629,12 @@ public static void clearCounts(final LevelChunkSection section) throws IllegalAc

public static BiomeType adapt(Holder<Biome> biome, LevelAccessor levelAccessor) {
final Registry<Biome> biomeRegistry = levelAccessor.registryAccess().registryOrThrow(BIOME);
if (biomeRegistry.getKey(biome.value()) == null) {
return biomeRegistry.asHolderIdMap().getId(biome) == -1 ? BiomeTypes.OCEAN
: null;
final int id = biomeRegistry.getId(biome.value());
if (id < 0) {
// this shouldn't be the case, but other plugins can be weird
return BiomeTypes.OCEAN;
}
return BiomeTypes.get(biome.unwrapKey().orElseThrow().location().toString());
return BiomeTypes.getLegacy(id);
}

static void removeBeacon(BlockEntity beacon, LevelChunk levelChunk) {
Expand Down
9 changes: 5 additions & 4 deletions ...java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_3/PaperweightPlatformAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,11 +636,12 @@ public static void clearCounts(final LevelChunkSection section) throws IllegalAc

public static BiomeType adapt(Holder<Biome> biome, LevelAccessor levelAccessor) {
final Registry<Biome> biomeRegistry = levelAccessor.registryAccess().lookupOrThrow(BIOME);
if (biomeRegistry.getKey(biome.value()) == null) {
return biomeRegistry.asHolderIdMap().getId(biome) == -1 ? BiomeTypes.OCEAN
: null;
final int id = biomeRegistry.getId(biome.value());
if (id < 0) {
// this shouldn't be the case, but other plugins can be weird
return BiomeTypes.OCEAN;
}
return BiomeTypes.get(biome.unwrapKey().orElseThrow().location().toString());
return BiomeTypes.getLegacy(id);
}

static void removeBeacon(BlockEntity beacon, LevelChunk levelChunk) {
Expand Down
9 changes: 5 additions & 4 deletions ...java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_21_4/PaperweightPlatformAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,11 +631,12 @@ public static void clearCounts(final LevelChunkSection section) throws IllegalAc

public static BiomeType adapt(Holder<Biome> biome, LevelAccessor levelAccessor) {
final Registry<Biome> biomeRegistry = levelAccessor.registryAccess().lookupOrThrow(BIOME);
if (biomeRegistry.getKey(biome.value()) == null) {
return biomeRegistry.asHolderIdMap().getId(biome) == -1 ? BiomeTypes.OCEAN
: null;
final int id = biomeRegistry.getId(biome.value());
if (id < 0) {
// this shouldn't be the case, but other plugins can be weird
return BiomeTypes.OCEAN;
}
return BiomeTypes.get(biome.unwrapKey().orElseThrow().location().toString());
return BiomeTypes.getLegacy(id);
}

static void removeBeacon(BlockEntity beacon, LevelChunk levelChunk) {
Expand Down
38 changes: 28 additions & 10 deletions worldedit-core/src/main/java/com/sk89q/worldedit/function/mask/BiomeMask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@
import javax.annotation.Nullable;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.Set;

import static com.google.common.base.Preconditions.checkNotNull;

Expand All @@ -38,7 +36,9 @@
public class BiomeMask extends AbstractExtentMask {
//FAWE end

private final Set<BiomeType> biomes = new HashSet<>();
//FAWE start - avoid HashSet usage
private final boolean[] biomes;
//FAWE end

/**
* Create a new biome mask.
Expand All @@ -51,7 +51,17 @@ public BiomeMask(Extent extent, Collection<BiomeType> biomes) {
super(extent);
//FAWE end
checkNotNull(biomes);
this.biomes.addAll(biomes);
//FAWE start - avoid HashSet usage
this.biomes = new boolean[BiomeType.REGISTRY.size()];
for (final BiomeType biome : biomes) {
this.biomes[biome.getInternalId()] = true;
}
//FAWE end
}

private BiomeMask(Extent extent, boolean[] biomes) {
super(extent);
this.biomes = biomes;
}

/**
Expand All @@ -71,7 +81,11 @@ public BiomeMask(Extent extent, BiomeType... biome) {
*/
public void add(Collection<BiomeType> biomes) {
checkNotNull(biomes);
this.biomes.addAll(biomes);
//FAWE start - avoid HashSet usage
for (final BiomeType biome : biomes) {
this.biomes[biome.getInternalId()] = true;
}
//FAWE end
}

/**
Expand All @@ -89,13 +103,17 @@ public void add(BiomeType... biome) {
* @return a list of biomes
*/
public Collection<BiomeType> getBiomes() {
return biomes;
//FAWE start - avoid HashSet usage
return BiomeType.REGISTRY.values().stream().filter(type -> biomes[type.getInternalId()]).toList();
//FAWE end
}

@Override
public boolean test(BlockVector3 vector) {
//FAWE start - avoid HashSet usage
BiomeType biome = vector.getBiome(getExtent());
return biomes.contains(biome);
return biomes[biome.getInternalId()];
//FAWE end
}

@Nullable
Expand All @@ -107,14 +125,14 @@ public Mask2D toMask2D() {
//FAWE start
@Override
public Mask copy() {
return new BiomeMask(getExtent(), new HashSet<>(biomes));
return new BiomeMask(getExtent(), this.biomes.clone());
}
//FAWE end

@Override
public boolean test(Extent extent, BlockVector3 position) {
BiomeType biome = getExtent().getBiome(position);
return biomes.contains(biome);
return biomes[biome.getInternalId()];
}
//FAWE end

}
17 changes: 14 additions & 3 deletions worldedit-core/src/main/java/com/sk89q/worldedit/world/biome/BiomeTypes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@
package com.sk89q.worldedit.world.biome;

import javax.annotation.Nullable;
import java.util.Arrays;
import java.util.Collection;
import java.util.Comparator;
import java.util.List;

/**
* Stores a list of common {@link BiomeType BiomeTypes}.
Expand Down Expand Up @@ -297,10 +300,18 @@ public static BiomeType register(final BiomeType biome) {
}

public static BiomeType getLegacy(int legacyId) {
for (BiomeType type : values()) {
if (type.getLegacyId() == legacyId) {
return type;
class BiomeTypeIdCache {
private static final List<BiomeType> byId = create();

private static List<BiomeType> create() {
BiomeType[] array = values().toArray(new BiomeType[0]);
Arrays.sort(array, Comparator.comparing(BiomeType::getLegacyId));
return List.of(array);
}

}
if (legacyId >= 0 && legacyId < BiomeTypeIdCache.byId.size()) {
return BiomeTypeIdCache.byId.get(legacyId);
Comment on lines +303 to +314
Copy link
Member

Choose a reason for hiding this comment

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

Any specific reason on using a List instead of an array?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really, the list is immutable and its backing array is marked stable which might help performance in some cases, but it likely doesn't matter.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think any performance drops from using a list should be compiled away given it's a static final list, so it should just act as an array in runtime

}
return null;
}
Expand Down
Loading