Skip to content
This repository has been archived by the owner on Feb 27, 2024. It is now read-only.

Commit

Permalink
Merge PhaseTracker changes to enable on-demand StackFrames. Improves …
Browse files Browse the repository at this point in the history
…performance overall.

Not likely to cause issues, but neighbor notifications are likely to be much faster as well.

Signed-off-by: Gabriel Harris-Rouquette <[email protected]>
  • Loading branch information
gabizou committed Jul 13, 2018
2 parents aea1580 + 8942eaa commit e37fa11
Show file tree
Hide file tree
Showing 16 changed files with 289 additions and 30 deletions.
2 changes: 1 addition & 1 deletion SpongeCommon
1 change: 1 addition & 0 deletions src/main/java/org/spongepowered/mod/SpongeCoremod.java
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public SpongeCoremod() {
Mixins.addConfiguration("mixins.forge.core.json");
Mixins.addConfiguration("mixins.forge.bungeecord.json");
Mixins.addConfiguration("mixins.forge.entityactivation.json");
Mixins.addConfiguration("mixins.forge.optimization.json");

MixinEnvironment.getDefaultEnvironment().registerTokenProviderClass("org.spongepowered.mod.SpongeCoremod$TokenProvider");

Expand Down
5 changes: 5 additions & 0 deletions src/main/java/org/spongepowered/mod/SpongeMod.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import net.minecraftforge.fml.relauncher.SideOnly;
import org.apache.logging.log4j.Level;
import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.objectweb.asm.Type;
import org.slf4j.Logger;
import org.spongepowered.api.Sponge;
Expand Down Expand Up @@ -129,6 +130,10 @@ public class SpongeMod extends MetaModContainer {

public static SpongeMod instance;

// USED ONLY TO KEEP TRACK OF THE THREAD. SINCE CLIENTS CAN HAVE MULTIPLE SERVERS
// WE NEED TO BE ABLE TO STORE A REFERENCE TO THE THREAD TO MAINTAIN SPEED OF ISMAINTHREAD CHECKS
@javax.annotation.Nullable public static Thread SERVVER_THREAD;

This comment has been minimized.

Copy link
@liach

liach Jul 14, 2018

This typo blows my eyes somehow. Reported to #263


@Inject private SpongeGame game;
@Inject private SpongeScheduler scheduler;
@Inject private Logger logger;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,7 @@ static Class<? extends net.minecraftforge.fml.common.eventhandler.Event> getForg
}

// ==================================== FORGE TO SPONGE START ==================================== \\
public static Event createSpongeEvent(net.minecraftforge.fml.common.eventhandler.Event forgeEvent,
CauseStackManager.StackFrame frame) { // TODO - maybe use the frame as a passed object, instead of having to create new frames.
public static Event createSpongeEvent(net.minecraftforge.fml.common.eventhandler.Event forgeEvent) { // TODO - maybe use the frame as a passed object, instead of having to create new frames.
return propgateCancellation(createSpongeEventImpl(forgeEvent), forgeEvent);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,15 @@
import org.spongepowered.asm.mixin.injection.Redirect;
import org.spongepowered.common.event.SpongeCommonEventFactory;
import org.spongepowered.common.event.tracking.IPhaseState;
import org.spongepowered.common.event.tracking.PhaseContext;
import org.spongepowered.common.event.tracking.PhaseTracker;
import org.spongepowered.common.event.tracking.phase.TrackingPhases;
import org.spongepowered.common.event.tracking.phase.block.BlockPhase;
import org.spongepowered.common.interfaces.world.IMixinWorldServer;
import org.spongepowered.common.mixin.core.block.MixinBlock;

import javax.annotation.Nullable;

@NonnullByDefault
@Mixin(value = BlockLog.class, priority = 1001)
public abstract class MixinBlockLog extends MixinBlock {
Expand All @@ -58,21 +61,28 @@ private void onSpongeBreakBlock(Block block, IBlockState state, net.minecraft.wo
final IPhaseState<?> currentState = phaseTracker.getCurrentState();
final boolean isBlockAlready = currentState.getPhase() != TrackingPhases.BLOCK;
final boolean isWorldGen = currentState.isWorldGeneration();
if (isBlockAlready && !isWorldGen) {
final LocatableBlock locatable = LocatableBlock.builder()
.location(new Location<>((World) worldIn, pos.getX(), pos.getY(), pos.getZ()))
.state((BlockState) state)
.build();
BlockPhase.State.BLOCK_DECAY.createPhaseContext()
.source(locatable)
.buildAndSwitch();
}
block.beginLeavesDecay(state, worldIn, pos);
if (isBlockAlready && !isWorldGen) {
phaseTracker.completePhase(BlockPhase.State.BLOCK_DECAY);
try (final PhaseContext<?> decayContext = createDecayContext((BlockState) state, (World) worldIn, pos, isBlockAlready && !isWorldGen)) {
if (decayContext != null) {
decayContext.buildAndSwitch();
}
block.beginLeavesDecay(state, worldIn, pos);
}
} else {
block.beginLeavesDecay(state, worldIn, pos);
}
}

@Nullable
private PhaseContext<?> createDecayContext(BlockState state, World worldIn, BlockPos pos, boolean canCreate) {
if (canCreate) {
final LocatableBlock locatable = LocatableBlock.builder()
.location(new Location<>(worldIn, pos.getX(), pos.getY(), pos.getZ()))
.state(state)
.build();
return BlockPhase.State.BLOCK_DECAY.createPhaseContext()
.source(locatable);
}
return null;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,18 @@ public static boolean isVanilla() {
return false;
}

/**
* @author gabizou - July 9th, 2018
* @reason During client shutdown or the integrated server shut down, we have
* to be able to detect the integrated server is shutting down and we should not
* be bothering with the phase tracker or cause stack manager.
* @return
*/
@Overwrite
public static boolean isClientAvailable() {
return FMLCommonHandler.instance().getSide().isClient();
}

/**
* @author unknown
* @reason Forge compatibility
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.Overwrite;
import org.spongepowered.asm.mixin.Shadow;
import org.spongepowered.common.SpongeImpl;
import org.spongepowered.common.SpongeImplHooks;
import org.spongepowered.common.interfaces.entity.IMixinVillager;
import org.spongepowered.common.mixin.core.entity.MixinEntityAgeable;
import org.spongepowered.mod.interfaces.IMixinEntityVillagerForge;
Expand Down Expand Up @@ -77,7 +77,7 @@ public void populateBuyingList() {
// Sponge - only get the profession once
final VillagerRegistry.VillagerCareer career = professionForge.getCareer(careerNumberId);
final IMixinVillagerCareer mixinCareer = (IMixinVillagerCareer) career;
if (mixinCareer.isDelayed() && SpongeImpl.isMainThread()) {
if (mixinCareer.isDelayed() && SpongeImplHooks.isMainThread()) {
mixinCareer.performDelayedInit();
}
if (mixinCareer.isModded()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,14 @@ public boolean post(Event event) {
public org.spongepowered.api.event.Event postForgeAndCreateSpongeEvent(Event forgeEvent) {
org.spongepowered.api.event.Event spongeEvent;
// Create a frame for the common event factory to push causes and contexts...
try (final CauseStackManager.StackFrame frame = Sponge.getCauseStackManager().pushCauseFrame()) {
spongeEvent = SpongeForgeEventFactory.createSpongeEvent(forgeEvent, frame); // todo : consider using the frame
// try (final CauseStackManager.StackFrame frame = Sponge.getCauseStackManager().pushCauseFrame()) {
spongeEvent = SpongeForgeEventFactory.createSpongeEvent(forgeEvent); // todo : consider using the frame
IEventListener[] listeners = forgeEvent.getListenerList().getListeners(this.busID);
boolean cancelled = ((SpongeModEventManager) SpongeImpl.getGame().getEventManager()).post(spongeEvent, forgeEvent, listeners, true);
if (!cancelled) {
SpongeForgeEventFactory.onForgePost(forgeEvent);
}
}
// }

return spongeEvent;
}
Expand All @@ -127,12 +127,12 @@ public boolean post(Event event, boolean forced) {
isSpongeSetUp = true;
// TODO verify the frame is necessary here or if it can be placed elsewhere
final boolean isMainThread = Sponge.isServerAvailable() && Sponge.getServer().isMainThread();
try (final CauseStackManager.StackFrame frame = isMainThread ? Sponge.getCauseStackManager().pushCauseFrame() : null) {
// try (final CauseStackManager.StackFrame frame = isMainThread ? Sponge.getCauseStackManager().pushCauseFrame() : null) {
if (!forced) {
if (!isEventAllowed(event)) {
return false;
}
spongeEvent = SpongeForgeEventFactory.createSpongeEvent(event, frame); // todo : consider using the frame
spongeEvent = SpongeForgeEventFactory.createSpongeEvent(event); // todo : consider using the frame
}

IEventListener[] listeners = event.getListenerList().getListeners(this.busID);
Expand Down Expand Up @@ -175,7 +175,7 @@ public boolean post(Event event, boolean forced) {
throw new RuntimeException(throwable);
}
return (event.isCancelable() ? event.isCanceled() : false);
}
// }
}

@SuppressWarnings({ "unchecked", "rawtypes" })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import net.minecraft.util.EnumHand;
import net.minecraft.util.math.BlockPos;
import net.minecraft.world.IBlockAccess;
import net.minecraft.world.World;
import net.minecraftforge.common.ForgeHooks;
import net.minecraftforge.common.MinecraftForge;
import net.minecraftforge.event.ForgeEventFactory;
Expand All @@ -43,11 +42,11 @@
import org.spongepowered.api.event.cause.EventContextKeys;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.Overwrite;
import org.spongepowered.common.SpongeImplHooks;
import org.spongepowered.common.event.SpongeCommonEventFactory;
import org.spongepowered.common.interfaces.world.IMixinWorld;
import org.spongepowered.common.interfaces.world.IMixinWorldServer;
import org.spongepowered.common.item.inventory.util.ItemStackUtil;
import org.spongepowered.common.util.ServerUtils;
import org.spongepowered.mod.interfaces.IMixinEventBus;

import javax.annotation.Nonnull;
Expand Down Expand Up @@ -105,7 +104,7 @@ public static boolean canHarvestBlock(@Nonnull Block block, @Nonnull EntityPlaye
}

// Sponge Start - Add the changeblockevent.pre check here before we bother with item stacks.
if (world instanceof IMixinWorldServer && !((IMixinWorld) world).isFake() && ServerUtils.isCallingFromMainThread()) {
if (world instanceof IMixinWorldServer && !((IMixinWorld) world).isFake() && SpongeImplHooks.isMainThread()) {
try (CauseStackManager.StackFrame frame = Sponge.getCauseStackManager().pushCauseFrame()) {
// Might as well provide the active item in use.
frame.addContext(EventContextKeys.USED_ITEM, ItemStackUtil.snapshotOf(player.getActiveItemStack()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@
import org.spongepowered.asm.mixin.Shadow;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Redirect;
import org.spongepowered.common.SpongeImplHooks;
import org.spongepowered.common.event.SpongeCommonEventFactory;
import org.spongepowered.common.interfaces.world.IMixinWorldServer;
import org.spongepowered.common.item.inventory.util.ItemStackUtil;
import org.spongepowered.common.mixin.core.world.MixinWorld;
import org.spongepowered.common.util.ServerUtils;
import org.spongepowered.common.world.gen.SpongeChunkGenerator;
import org.spongepowered.common.world.gen.SpongeWorldGenerator;
import org.spongepowered.common.world.storage.SpongeChunkLayout;
Expand Down Expand Up @@ -95,7 +95,7 @@ private boolean isSpongeBlockProtected(MinecraftServer server, net.minecraft.wor
if (server.isBlockProtected(worldIn, pos, playerIn)) {
return true;
}
if (!this.isFake() && ServerUtils.isCallingFromMainThread()) {
if (!this.isFake() && SpongeImplHooks.isMainThread()) {
try (CauseStackManager.StackFrame frame = Sponge.getCauseStackManager().pushCauseFrame()) {
// Might as well provide the active item in use.
frame.addContext(EventContextKeys.USED_ITEM, ItemStackUtil.snapshotOf(playerIn.getActiveItemStack()));
Expand All @@ -119,7 +119,7 @@ public boolean isBlockModifiable(EntityPlayer player, BlockPos pos) {
if (super.isBlockModifiable(player, pos)) {
return true;
}
if (!this.isFake() && ServerUtils.isCallingFromMainThread()) {
if (!this.isFake() && SpongeImplHooks.isMainThread()) {
try (CauseStackManager.StackFrame frame = Sponge.getCauseStackManager().pushCauseFrame()) {
// Might as well provide the active item in use.
frame.addContext(EventContextKeys.USED_ITEM, ItemStackUtil.snapshotOf(player.getActiveItemStack()));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* This file is part of Sponge, licensed under the MIT License (MIT).
*
* Copyright (c) SpongePowered <https://www.spongepowered.org>
* Copyright (c) contributors
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.spongepowered.mod.mixin.optimization.threadchecks;

import net.minecraft.server.integrated.IntegratedServer;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
import org.spongepowered.mod.SpongeMod;

@Mixin(IntegratedServer.class)
public class MixinIntegratedServer_ThreadChecks {

@Inject(method = "stopServer", at = @At(value = "INVOKE", target = "Lnet/minecraft/server/MinecraftServer;stopServer()V", shift = At.Shift.AFTER))
private void onSpongeShutDown(CallbackInfo callbackInfo) {
SpongeMod.SERVVER_THREAD = null; // Reset it
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* This file is part of Sponge, licensed under the MIT License (MIT).
*
* Copyright (c) SpongePowered <https://www.spongepowered.org>
* Copyright (c) contributors
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.spongepowered.mod.mixin.optimization.threadchecks;

import net.minecraft.server.MinecraftServer;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Redirect;
import org.spongepowered.mod.SpongeMod;

@Mixin(MinecraftServer.class)
public class MixinMinecraftServer_ThreadChecks {

@Redirect(method = "startServerThread", at = @At(value = "INVOKE", target = "Ljava/lang/Thread;start()V", remap = false))
private void onServerStart(Thread thread) {
SpongeMod.SERVVER_THREAD = thread;
thread.start();

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* This file is part of Sponge, licensed under the MIT License (MIT).
*
* Copyright (c) SpongePowered <https://www.spongepowered.org>
* Copyright (c) contributors
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.spongepowered.mod.mixin.optimization.threadchecks;


import net.minecraft.server.MinecraftServer;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.Overwrite;
import org.spongepowered.common.SpongeImplHooks;
import org.spongepowered.mod.SpongeMod;

@Mixin(value = SpongeImplHooks.class, remap = false)
public class MixinSpongeImplHooks_ThreadChecks {

/**
* @author gabizou - July 8th, 2018
* @reason Makes the thread checks simply faster than the multiple
* null and state checks and finally delegating to
* {@link MinecraftServer#isCallingFromMinecraftThread()}. This is
* only possible through mixins due to having to set and reset the
* server thread.
*/
@Overwrite
public static boolean isMainThread() {
// Return true when the server isn't yet initialized, this means on a client
// that the game is still being loaded. This is needed to support initialization
// events with cause tracking.
return SpongeMod.SERVVER_THREAD == null || Thread.currentThread() == SpongeMod.SERVVER_THREAD;

This comment has been minimized.

Copy link
@JBYoshi

JBYoshi Jul 17, 2018

Member

Will this still work when the client has finished starting up but the integrated server is not running (such as when the client is connected to another server)?

This comment has been minimized.

Copy link
@gabizou

gabizou Jul 17, 2018

Author Member

I'm fairly sure it will work. I'm going to do some testing right now to verify.

}
}
Loading

0 comments on commit e37fa11

Please sign in to comment.