server micro optimizations #194
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| --- a/net/minecraft/world/entity/npc/villager/Villager.java | ||
| +++ b/net/minecraft/world/entity/npc/villager/Villager.java | ||
| @@ -887,7 +_,11 @@ | ||
| if (this.lastGossipDecayTime == 0L) { | ||
| this.lastGossipDecayTime = timestamp; | ||
| } else if (timestamp >= this.lastGossipDecayTime + 24000L) { | ||
| - this.gossips.decay(); | ||
| + // Gale start - Do less work - Skip gossip decay for villagers without any gossip | ||
| + if (!this.gossips.gossips.isEmpty()) { | ||
| + this.gossips.decay(); | ||
|
Member
There was a problem hiding this comment. Move line one tab the left is preferred since we have a proximity diff on it anyway
Member
Author
There was a problem hiding this comment. Done - moved it one tab left so the wrapped line stays at its original indentation. |
||
| + } | ||
| + // Gale end - Do less work - Skip gossip decay for villagers without any gossip | ||
| this.lastGossipDecayTime = timestamp; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| --- a/net/minecraft/world/level/BaseSpawner.java | ||
| +++ b/net/minecraft/world/level/BaseSpawner.java | ||
| @@ -106,7 +_,7 @@ | ||
| boolean delay = false; | ||
| RandomSource random = level.getRandom(); | ||
| SpawnData nextSpawnData = this.getOrCreateNextSpawnData(level, random, pos); | ||
| - | ||
|
Member
Author
There was a problem hiding this comment. Restored. RIP empty line 2026-2026 xD |
||
| + AABB nearbyEntitiesBox = new AABB(pos.getX(), pos.getY(), pos.getZ(), pos.getX() + 1, pos.getY() + 1, pos.getZ() + 1).inflate(this.spawnRange); // Gale - Do less work - Hoist nearby entity query box out of the spawn loop | ||
| for (int c = 0; c < this.spawnCount; c++) { | ||
| try (ProblemReporter.ScopedCollector reporter = new ProblemReporter.ScopedCollector(this::toString, LOGGER)) { | ||
| ValueInput input = TagValueInput.create(reporter, level.registryAccess(), nextSpawnData.getEntityToSpawn()); | ||
| @@ -165,7 +_,7 @@ | ||
|
|
||
| int nearBy = level.getEntities( | ||
| EntityTypeTest.forExactClass(entity.getClass()), | ||
| - new AABB(pos.getX(), pos.getY(), pos.getZ(), pos.getX() + 1, pos.getY() + 1, pos.getZ() + 1).inflate(this.spawnRange), | ||
| + nearbyEntitiesBox, // Gale - Do less work - Hoist nearby entity query box out of the spawn loop | ||
| EntitySelector.NO_SPECTATORS | ||
| ) | ||
| .size(); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -288,6 +288,33 @@ | |
| this.tickingBlockEntities = true; | ||
| if (!this.pendingBlockEntityTickers.isEmpty()) { | ||
| this.blockEntityTickers.addAll(this.pendingBlockEntityTickers); | ||
| @@ -1498,11 +_,13 @@ | ||
| // Paper start - Fix MC-117075 use removeAll | ||
| final it.unimi.dsi.fastutil.objects.ReferenceOpenHashSet<@Nullable TickingBlockEntity> toRemove = new it.unimi.dsi.fastutil.objects.ReferenceOpenHashSet<>(); | ||
| toRemove.add(null); | ||
|
Member
There was a problem hiding this comment. There must be a reason for the
Member
Author
There was a problem hiding this comment. That line is Paper's, not ours - it's part of Paper's MC-117075 fix (blockEntityTickers can contain nulls, so null is added to the reference set so removeAll strips them too). It only appears as an addition here because our removeAny change pulled that region into the patch as context. Our change in this method is just the removeAny guard.
Collaborator
There was a problem hiding this comment.
I don't know why they add this This line of change was added in this commit PaperMC/Paper-archive@6f064f9#diff-477840aa089f974ea049572e2fc59e5dec986abc597ea9b1ce0f1487caad587c And Machine Maker didn't leave a note to explain why he added If the |
||
| + boolean removeAny = false; // Gale - Do less work - Skip the removeAll scan when nothing was removed | ||
| for (int tickerIndex = 0; tickerIndex < this.blockEntityTickers.size(); tickerIndex++) { | ||
| final TickingBlockEntity ticker = this.blockEntityTickers.get(tickerIndex); | ||
| // Paper end - Fix MC-117075 use removeAll | ||
| if (ticker.isRemoved()) { | ||
| toRemove.add(ticker); // Paper - Fix MC-117075 use removeAll | ||
| + removeAny = true; // Gale - Do less work - Skip the removeAll scan when nothing was removed | ||
| } else if (tickBlockEntities && this.shouldTickBlocksAt(ticker.getPos())) { | ||
| ticker.tick(); | ||
| // Paper start - rewrite chunk system | ||
| @@ -1513,7 +_,11 @@ | ||
| } | ||
| } | ||
|
|
||
| - this.blockEntityTickers.removeAll(toRemove); // Paper - Fix MC-117075 use removeAll | ||
| + // Gale start - Do less work - Skip the removeAll scan when nothing was removed | ||
| + if (removeAny) { | ||
| + this.blockEntityTickers.removeAll(toRemove); // Paper - Fix MC-117075 use removeAll | ||
| + } | ||
| + // Gale end - Do less work - Skip the removeAll scan when nothing was removed | ||
| this.tickingBlockEntities = false; | ||
| } | ||
|
|
||
| @@ -2127,6 +_,13 @@ | ||
| public BiomeManager getBiomeManager() { | ||
| return this.biomeManager; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| --- a/net/minecraft/world/level/block/entity/BeehiveBlockEntity.java | ||
| +++ b/net/minecraft/world/level/block/entity/BeehiveBlockEntity.java | ||
| @@ -335,6 +_,7 @@ | ||
| } | ||
|
|
||
| public static void serverTick(final Level level, final BlockPos blockPos, final BlockState state, final BeehiveBlockEntity entity) { | ||
| + if (entity.stored.isEmpty()) return; // Gale - Do less work - Skip bee hive ticking when it holds no bees | ||
| tickOccupants(level, blockPos, state, entity.stored, entity.savedFlowerPos); | ||
| if (!entity.stored.isEmpty() && level.getRandom().nextDouble() < 0.005) { | ||
| double x = blockPos.getX() + 0.5; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| --- a/net/minecraft/world/level/block/entity/HopperBlockEntity.java | ||
| +++ b/net/minecraft/world/level/block/entity/HopperBlockEntity.java | ||
| @@ -38,6 +_,8 @@ | ||
| public int cooldownTime = -1; | ||
| private long tickedGameTime; | ||
| private Direction facing; | ||
| + private @Nullable AABB cachedSuckAabb; // Gale - Do less work - Cache hopper suck AABB | ||
| + private @Nullable BlockPos cachedSuckPos; // Gale - Do less work - Cache hopper suck AABB (invalidate on position change) | ||
|
|
||
| // CraftBukkit start - add fields and methods | ||
| public List<org.bukkit.entity.HumanEntity> transaction = new java.util.ArrayList<>(); | ||
| @@ -668,7 +_,9 @@ | ||
| } | ||
|
|
||
| public static List<ItemEntity> getItemsAtAndAbove(final Level level, final Hopper hopper) { | ||
| - AABB aabb = hopper.getSuckAabb().move(hopper.getLevelX() - 0.5, hopper.getLevelY() - 0.5, hopper.getLevelZ() - 0.5); | ||
| + AABB aabb = hopper instanceof HopperBlockEntity hopperBlockEntity // Gale - Do less work - Cache hopper suck AABB | ||
| + ? hopperBlockEntity.gale$getCachedSuckAabb() | ||
| + : hopper.getSuckAabb().move(hopper.getLevelX() - 0.5, hopper.getLevelY() - 0.5, hopper.getLevelZ() - 0.5); | ||
| return level.getEntitiesOfClass(ItemEntity.class, aabb, EntitySelector.ENTITY_STILL_ALIVE); | ||
| } | ||
|
|
||
| @@ -742,6 +_,18 @@ | ||
| return true; | ||
| } | ||
|
|
||
| + // Gale start - Do less work - Cache hopper suck AABB | ||
| + public AABB gale$getCachedSuckAabb() { | ||
| + AABB aabb = this.cachedSuckAabb; | ||
| + BlockPos pos = this.getBlockPos(); | ||
| + if (aabb == null || !pos.equals(this.cachedSuckPos)) { | ||
| + this.cachedSuckAabb = aabb = this.getSuckAabb().move(this.getLevelX() - 0.5, this.getLevelY() - 0.5, this.getLevelZ() - 0.5); | ||
| + this.cachedSuckPos = pos; | ||
| + } | ||
| + return aabb; | ||
| + } | ||
| + // Gale end - Do less work - Cache hopper suck AABB | ||
| + | ||
| public void setCooldown(final int time) { | ||
| this.cooldownTime = time; | ||
| } | ||
| @@ -767,7 +_,7 @@ | ||
| public static void entityInside(final Level level, final BlockPos pos, final BlockState blockState, final Entity entity, final HopperBlockEntity hopper) { | ||
| if (entity instanceof ItemEntity itemEntity | ||
| && !itemEntity.getItem().isEmpty() | ||
| - && entity.getBoundingBox().move(-pos.getX(), -pos.getY(), -pos.getZ()).intersects(hopper.getSuckAabb())) { | ||
| + && entity.getBoundingBox().intersects(hopper.gale$getCachedSuckAabb())) { // Gale - Do less work - Cache hopper suck AABB, avoids AABB allocation per item entity | ||
|
Member
There was a problem hiding this comment. I haven't done the logic in my head to figure out whether this is actually the same. Is it?
Member
Author
There was a problem hiding this comment. Yes - it's the same, by translation invariance of AABB.intersects. gale() returns getSuckAabb().move(getLevelX() - 0.5, getLevelY() - 0.5, getLevelZ() - 0.5), and for a HopperBlockEntity getLevelX()/getLevelY()/getLevelZ() are worldPosition + 0.5, so the cache holds suckAabb.move(pos) in world coordinates. A.intersects(B) <=> A.move(v).intersects(B.move(v)) for any v, since both boxes shift by the same vector and the min/max comparisons cancel out. So box.move(-pos).intersects(suckAabb) <=> box.intersects(suckAabb.move(pos)), which is exactly what the cached path checks. (The non-HopperBlockEntity path, i.e. MinecartHopper, still uses the exact vanilla expression.) |
||
| tryMoveItems(level, pos, blockState, hopper, () -> addItem(hopper, itemEntity)); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is completely pointless.
In case of InactiveProfiler,
pushandpopare no-op, so they won't have any effect.And JIT will quickly realize (during a run where profiler is disabled, which is essentially always) that there is only 1 runtime instance of profiler and it will completely delete these lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, removed it - reverted to the vanilla push/pop. The JIT devirtualizes InactiveProfiler and eliminates these branches when profiling is off anyway.