Don't run CI on 1.12

Our cable has a rather complex getCollisionBoundingBox implementation,
which (combined with computing the actual block state) ends up taking an
awful lot of time.

This shaves 9% off each frame. I don't miss 1.12 one bit.

.github/workflows/main-ci.yml

@@ -1,53 +0,0 @@
-name: Build
-
-on: [push, pull_request]
-
-jobs:
-  build:
-    name: Build
-    runs-on: ubuntu-latest
-
-    steps:
-    - uses: actions/checkout@v1
-
-    - name: Set up JDK 1.8
-      uses: actions/setup-java@v1
-      with:
-        java-version: 1.8
-
-    - name: Cache gradle dependencies
-      uses: actions/cache@v1
-      with:
-        path: ~/.gradle/caches
-        key: ${{ runner.os }}-gradle-${{ hashFiles('gradle.properties') }}
-        restore-keys: |
-          ${{ runner.os }}-gradle-
-
-    - name: Build with Gradle
-      run: ./gradlew build --no-daemon
-
-    - name: Upload Jar
-      uses: actions/upload-artifact@v1
-      with:
-        name: CC-Tweaked
-        path: build/libs
-
-    - name: Upload Coverage
-      run: bash <(curl -s https://codecov.io/bash)
-
-  lint-lua:
-    name: Lint Lua
-    runs-on: ubuntu-latest
-
-    steps:
-    - uses: actions/checkout@v1
-
-    - name: Lint Lua code
-      run: |
-        test -d bin || mkdir bin
-        test -f bin/illuaminate || wget -q -Obin/illuaminate https://squiddev.cc/illuaminate/linux-x86-64/illuaminate
-        chmod +x bin/illuaminate
-        bin/illuaminate lint
-
-    - name: Check whitespace
-      run: python3 tools/check-lines.py

.github/workflows/make-doc.sh

@@ -1,16 +0,0 @@
-#!/usr/bin/env bash
-
-set -eu
-
-DEST="${GITHUB_REF#refs/*/}"
-echo "Uploading docs to https://tweaked.cc/$DEST"
-
-# Setup ssh key
-mkdir -p "$HOME/.ssh/"
-echo "$SSH_KEY" > "$HOME/.ssh/key"
-chmod 600 "$HOME/.ssh/key"
-
-# And upload
-rsync -avc -e "ssh -i $HOME/.ssh/key -o StrictHostKeyChecking=no -p $SSH_PORT" \
-      "$GITHUB_WORKSPACE/doc/" \
-      "$SSH_USER@$SSH_HOST:/var/www/tweaked.cc/$DEST"

.github/workflows/make-doc.yml

@@ -1,29 +0,0 @@
-name: Build documentation
-
-on:
-  push:
-    branches: [ master ]
-    tags:
-
-jobs:
-  make_doc:
-    name: Build
-    runs-on: ubuntu-latest
-
-    steps:
-    - uses: actions/checkout@v1
-
-    - name: Build documentation
-      run: |
-        test -d bin || mkdir bin
-        test -f bin/illuaminate || wget -q -Obin/illuaminate https://squiddev.cc/illuaminate/linux-x86-64/illuaminate
-        chmod +x bin/illuaminate
-        bin/illuaminate doc-gen
-
-    - name: Upload documentation
-      run: .github/workflows/make-doc.sh 2> /dev/null
-      env:
-        SSH_KEY:  ${{ secrets.SSH_KEY  }}
-        SSH_USER: ${{ secrets.SSH_USER }}
-        SSH_HOST: ${{ secrets.SSH_HOST }}
-        SSH_PORT: ${{ secrets.SSH_PORT }}

build.gradle

@@ -52,14 +52,6 @@ repositories {
         name "Charset"
         artifactPattern "https://asie.pl/files/mods/Charset/LibOnly/[module]-[revision](-[classifier]).[ext]"
     }
-    maven {
-        name "Amadornes"
-        url "https://maven.amadornes.com/"
-    }
-    maven {
-        name "CraftTweaker"
-        url "https://maven.blamejared.com/"
-    }
 }
 
 configurations {
@@ -78,7 +70,7 @@ dependencies {
 
     runtime "mezz.jei:jei_1.12.2:4.15.0.269"
 
-    shade 'org.squiddev:Cobalt:0.5.1-SNAPSHOT'
+    shade 'org.squiddev:Cobalt:0.5.5'
 
     testImplementation 'org.junit.jupiter:junit-jupiter-api:5.4.2'
     testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.4.2'
@@ -144,6 +136,9 @@ task proguard(type: ProGuardTask, dependsOn: jar) {
     // We want to avoid as much obfuscation as possible. We're only doing this to shrink code size.
     dontobfuscate; dontoptimize; keepattributes; keepparameternames
 
+    // Tell ProGuard to shut up
+    dontwarn 'org.checkerframework.**'
+
     // Proguard will remove directories by default, but that breaks JarMount.
     keepdirectories 'assets/computercraft/lua**'
 
@@ -265,7 +260,7 @@ license {
     mapping("java", "SLASHSTAR_STYLE")
     strictCheck true
 
-    ext.year = Calendar.getInstance().get(Calendar.YEAR)
+    ext.year = 2020
 }
 
 [licenseMain, licenseFormatMain].forEach {

src/main/java/dan200/computercraft/core/computer/ComputerThread.java

@@ -13,6 +13,7 @@ import javax.annotation.Nullable;
 import java.util.TreeSet;
 import java.util.concurrent.ThreadFactory;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.locks.Condition;
 import java.util.concurrent.locks.LockSupport;
@@ -49,11 +50,11 @@ import static dan200.computercraft.core.computer.TimeoutState.TIMEOUT;
 public final class ComputerThread
 {
     /**
-     * How often the computer thread monitor should run, in milliseconds.
+     * How often the computer thread monitor should run.
      *
      * @see Monitor
      */
-    private static final int MONITOR_WAKEUP = 100;
+    private static final long MONITOR_WAKEUP = TimeUnit.MILLISECONDS.toNanos( 100 );
 
     /**
      * The target latency between executing two tasks on a single machine.
@@ -76,6 +77,13 @@ public final class ComputerThread
      */
     private static final long LATENCY_MAX_TASKS = DEFAULT_LATENCY / DEFAULT_MIN_PERIOD;
 
+    /**
+     * Time difference between reporting crashed threads.
+     *
+     * @see TaskRunner#reportTimeout(ComputerExecutor, long)
+     */
+    private static final long REPORT_DEBOUNCE = TimeUnit.SECONDS.toNanos( 1 );
+
     /**
      * Lock used for modifications to the array of current threads.
      */
@@ -102,6 +110,8 @@ public final class ComputerThread
     private static final ReentrantLock computerLock = new ReentrantLock();
 
     private static final Condition hasWork = computerLock.newCondition();
+    private static final AtomicInteger idleWorkers = new AtomicInteger( 0 );
+    private static final Condition monitorWakeup = computerLock.newCondition();
 
     /**
      * Active queues to execute.
@@ -135,7 +145,7 @@ public final class ComputerThread
 
             if( runners == null )
             {
-                // TODO: Change the runners length on config reloads
+                // TODO: Update this on config reloads. Or possibly on world restarts?
                 runners = new TaskRunner[ComputerCraft.computer_threads];
 
                 // latency and minPeriod are scaled by 1 + floor(log2(threads)). We can afford to execute tasks for
@@ -227,9 +237,14 @@ public final class ComputerThread
 
             executor.virtualRuntime = Math.max( newRuntime, executor.virtualRuntime );
 
+            boolean wasBusy = isBusy();
             // Add to the queue, and signal the workers.
             computerQueue.add( executor );
             hasWork.signal();
+
+            // If we've transitioned into a busy state, notify the monitor. This will cause it to sleep for scaledPeriod
+            // instead of the longer wakeup duration.
+            if( !wasBusy && isBusy() ) monitorWakeup.signal();
         }
         finally
         {
@@ -346,6 +361,17 @@ public final class ComputerThread
         return !computerQueue.isEmpty();
     }
 
+    /**
+     * Check if we have more work queued than we have capacity for. Effectively a more fine-grained version of
+     * {@link #hasPendingWork()}.
+     *
+     * @return If the computer threads are busy.
+     */
+    private static boolean isBusy()
+    {
+        return computerQueue.size() > idleWorkers.get();
+    }
+
     /**
      * Observes all currently active {@link TaskRunner}s and terminates their tasks once they have exceeded the hard
      * abort limit.
@@ -357,76 +383,93 @@ public final class ComputerThread
         @Override
         public void run()
         {
-            try
+            while( true )
             {
-                while( true )
+                computerLock.lock();
+                try
                 {
-                    Thread.sleep( MONITOR_WAKEUP );
+                    // If we've got more work than we have capacity for it, then we'll need to pause a task soon, so
+                    // sleep for a single pause duration. Otherwise we only need to wake up to set the soft/hard abort
+                    // flags, which are far less granular.
+                    monitorWakeup.awaitNanos( isBusy() ? scaledPeriod() : MONITOR_WAKEUP );
+                }
+                catch( InterruptedException e )
+                {
+                    ComputerCraft.log.error( "Monitor thread interrupted. Computers may behave very badly!", e );
+                    break;
+                }
+                finally
+                {
+                    computerLock.unlock();
+                }
 
-                    TaskRunner[] currentRunners = ComputerThread.runners;
-                    if( currentRunners != null )
+                checkRunners();
+            }
+        }
+
+        private static void checkRunners()
+        {
+            TaskRunner[] currentRunners = ComputerThread.runners;
+            if( currentRunners == null ) return;
+
+            for( int i = 0; i < currentRunners.length; i++ )
+            {
+                TaskRunner runner = currentRunners[i];
+                // If we've no runner, skip.
+                if( runner == null || runner.owner == null || !runner.owner.isAlive() )
+                {
+                    if( !running ) continue;
+
+                    // Mark the old runner as dead and start a new one.
+                    ComputerCraft.log.warn( "Previous runner ({}) has crashed, restarting!",
+                        runner != null && runner.owner != null ? runner.owner.getName() : runner );
+                    if( runner != null ) runner.running = false;
+                    runnerFactory.newThread( runners[i] = new TaskRunner() ).start();
+                }
+
+                // If the runner has no work, skip
+                ComputerExecutor executor = runner.currentExecutor.get();
+                if( executor == null ) continue;
+
+                // Refresh the timeout state. Will set the pause/soft timeout flags as appropriate.
+                executor.timeout.refresh();
+
+                // If we're still within normal execution times (TIMEOUT) or soft abort (ABORT_TIMEOUT),
+                // then we can let the Lua machine do its work.
+                long afterStart = executor.timeout.nanoCumulative();
+                long afterHardAbort = afterStart - TIMEOUT - ABORT_TIMEOUT;
+                if( afterHardAbort < 0 ) continue;
+
+                // Set the hard abort flag.
+                executor.timeout.hardAbort();
+                executor.abort();
+
+                if( afterHardAbort >= ABORT_TIMEOUT * 2 )
+                {
+                    // If we've hard aborted and interrupted, and we're still not dead, then mark the runner
+                    // as dead, finish off the task, and spawn a new runner.
+                    runner.reportTimeout( executor, afterStart );
+                    runner.running = false;
+                    runner.owner.interrupt();
+
+                    ComputerExecutor thisExecutor = runner.currentExecutor.getAndSet( null );
+                    if( thisExecutor != null ) afterWork( runner, executor );
+
+                    synchronized( threadLock )
                     {
-                        for( int i = 0; i < currentRunners.length; i++ )
+                        if( running && runners.length > i && runners[i] == runner )
                         {
-                            TaskRunner runner = currentRunners[i];
-                            // If we've no runner, skip.
-                            if( runner == null || runner.owner == null || !runner.owner.isAlive() )
-                            {
-                                if( !running ) continue;
-
-                                // Mark the old runner as dead and start a new one.
-                                ComputerCraft.log.warn( "Previous runner ({}) has crashed, restarting!",
-                                    runner != null && runner.owner != null ? runner.owner.getName() : runner );
-                                if( runner != null ) runner.running = false;
-                                runnerFactory.newThread( runners[i] = new TaskRunner() ).start();
-                            }
-
-                            // If the runner has no work, skip
-                            ComputerExecutor executor = runner.currentExecutor.get();
-                            if( executor == null ) continue;
-
-                            // If we're still within normal execution times (TIMEOUT) or soft abort (ABORT_TIMEOUT),
-                            // then we can let the Lua machine do its work.
-                            long afterStart = executor.timeout.nanoCumulative();
-                            long afterHardAbort = afterStart - TIMEOUT - ABORT_TIMEOUT;
-                            if( afterHardAbort < 0 ) continue;
-
-                            // Set the hard abort flag.
-                            executor.timeout.hardAbort();
-                            executor.abort();
-
-                            if( afterHardAbort >= ABORT_TIMEOUT )
-                            {
-                                // If we've hard aborted but we're still not dead, dump the stack trace and interrupt
-                                // the task.
-                                timeoutTask( executor, runner.owner, afterStart );
-                                runner.owner.interrupt();
-                            }
-                            else if( afterHardAbort >= ABORT_TIMEOUT * 2 )
-                            {
-                                // If we've hard aborted and interrupted, and we're still not dead, then mark the runner
-                                // as dead, finish off the task, and spawn a new runner.
-                                timeoutTask( executor, runner.owner, afterStart );
-                                runner.running = false;
-                                runner.owner.interrupt();
-
-                                ComputerExecutor thisExecutor = runner.currentExecutor.getAndSet( null );
-                                if( thisExecutor != null ) afterWork( runner, executor );
-
-                                synchronized( threadLock )
-                                {
-                                    if( running && runners.length > i && runners[i] == runner )
-                                    {
-                                        runnerFactory.newThread( currentRunners[i] = new TaskRunner() ).start();
-                                    }
-                                }
-                            }
+                            runnerFactory.newThread( currentRunners[i] = new TaskRunner() ).start();
                         }
                     }
                 }
-            }
-            catch( InterruptedException ignored )
-            {
+                else if( afterHardAbort >= ABORT_TIMEOUT )
+                {
+                    // If we've hard aborted but we're still not dead, dump the stack trace and interrupt
+                    // the task.
+                    runner.reportTimeout( executor, afterStart );
+                    runner.owner.interrupt();
+                }
             }
         }
     }
@@ -441,6 +484,7 @@ public final class ComputerThread
     private static final class TaskRunner implements Runnable
     {
         Thread owner;
+        long lastReport = Long.MIN_VALUE;
         volatile boolean running = true;
 
         final AtomicReference<ComputerExecutor> currentExecutor = new AtomicReference<>();
@@ -460,6 +504,7 @@ public final class ComputerThread
                     computerLock.lockInterruptibly();
                     try
                     {
+                        idleWorkers.incrementAndGet();
                         while( computerQueue.isEmpty() ) hasWork.await();
                         executor = computerQueue.pollFirst();
                         assert executor != null : "hasWork should ensure we never receive null work";
@@ -467,6 +512,7 @@ public final class ComputerThread
                     finally
                     {
                         computerLock.unlock();
+                        idleWorkers.decrementAndGet();
                     }
                 }
                 catch( InterruptedException ignored )
@@ -514,27 +560,32 @@ public final class ComputerThread
                 }
             }
         }
-    }
 
-    private static void timeoutTask( ComputerExecutor executor, Thread thread, long time )
-    {
-        if( !ComputerCraft.logPeripheralErrors ) return;
-
-        StringBuilder builder = new StringBuilder()
-            .append( "Terminating computer #" ).append( executor.getComputer().getID() )
-            .append( " due to timeout (running for " ).append( time * 1e-9 )
-            .append( " seconds). This is NOT a bug, but may mean a computer is misbehaving. " )
-            .append( thread.getName() )
-            .append( " is currently " )
-            .append( thread.getState() );
-        Object blocking = LockSupport.getBlocker( thread );
-        if( blocking != null ) builder.append( "\n  on " ).append( blocking );
-
-        for( StackTraceElement element : thread.getStackTrace() )
+        private void reportTimeout( ComputerExecutor executor, long time )
         {
-            builder.append( "\n  at " ).append( element );
-        }
+            if( !ComputerCraft.logPeripheralErrors ) return;
 
-        ComputerCraft.log.warn( builder.toString() );
+            // Attempt to debounce stack trace reporting, limiting ourselves to one every second.
+            long now = System.nanoTime();
+            if( lastReport != Long.MIN_VALUE && now - lastReport - REPORT_DEBOUNCE <= 0 ) return;
+            lastReport = now;
+
+            StringBuilder builder = new StringBuilder()
+                .append( "Terminating computer #" ).append( executor.getComputer().getID() )
+                .append( " due to timeout (running for " ).append( time * 1e-9 )
+                .append( " seconds). This is NOT a bug, but may mean a computer is misbehaving. " )
+                .append( owner.getName() )
+                .append( " is currently " )
+                .append( owner.getState() );
+            Object blocking = LockSupport.getBlocker( owner );
+            if( blocking != null ) builder.append( "\n  on " ).append( blocking );
+
+            for( StackTraceElement element : owner.getStackTrace() )
+            {
+                builder.append( "\n  at " ).append( element );
+            }
+
+            ComputerCraft.log.warn( builder.toString() );
+        }
     }
 }

src/main/java/dan200/computercraft/core/computer/TimeoutState.java

@@ -86,7 +86,7 @@ public final class TimeoutState
     /**
      * Recompute the {@link #isSoftAborted()} and {@link #isPaused()} flags.
      */
-    public void refresh()
+    public synchronized void refresh()
     {
         // Important: The weird arithmetic here is important, as nanoTime may return negative values, and so we
         // need to handle overflow.
@@ -153,7 +153,7 @@ public final class TimeoutState
      *
      * @see #nanoCumulative()
      */
-    void pauseTimer()
+    synchronized void pauseTimer()
     {
         // We set the cumulative time to difference between current time and "nominal start time".
         cumulativeElapsed = System.nanoTime() - cumulativeStart;
@@ -163,7 +163,7 @@ public final class TimeoutState
     /**
      * Resets the cumulative time and resets the abort flags.
      */
-    void stopTimer()
+    synchronized void stopTimer()
     {
         cumulativeElapsed = 0;
         paused = softAbort = hardAbort = false;

src/main/java/dan200/computercraft/core/lua/CobaltLuaMachine.java

@@ -445,24 +445,9 @@ public class CobaltLuaMachine implements ILuaMachine
             // We check our current pause/abort state every 128 instructions.
             if( (count = (count + 1) & 127) == 0 )
             {
-                // If we've been hard aborted or closed then abort.
                 if( timeout.isHardAborted() || m_state == null ) throw HardAbortError.INSTANCE;
-
-                timeout.refresh();
-                if( timeout.isPaused() )
-                {
-                    // Preserve the current state
-                    isPaused = true;
-                    oldInHook = ds.inhook;
-                    oldFlags = di.flags;
-
-                    // Suspend the state. This will probably throw, but we need to handle the case where it won't.
-                    di.flags |= FLAG_HOOKYIELD | FLAG_HOOKED;
-                    LuaThread.suspend( ds.getLuaState() );
-                    resetPaused( ds, di );
-                }
-
-                handleSoftAbort();
+                if( timeout.isPaused() ) handlePause( ds, di );
+                if( timeout.isSoftAborted() ) handleSoftAbort();
             }
 
             super.onInstruction( ds, di, pc );
@@ -471,13 +456,10 @@ public class CobaltLuaMachine implements ILuaMachine
         @Override
         public void poll() throws LuaError
         {
-            // If we've been hard aborted or closed then abort.
             LuaState state = m_state;
             if( timeout.isHardAborted() || state == null ) throw HardAbortError.INSTANCE;
-
-            timeout.refresh();
             if( timeout.isPaused() ) LuaThread.suspendBlocking( state );
-            handleSoftAbort();
+            if( timeout.isSoftAborted() ) handleSoftAbort();
         }
 
         private void resetPaused( DebugState ds, DebugFrame di )
@@ -491,11 +473,24 @@ public class CobaltLuaMachine implements ILuaMachine
         private void handleSoftAbort() throws LuaError
         {
             // If we already thrown our soft abort error then don't do it again.
-            if( !timeout.isSoftAborted() || thrownSoftAbort ) return;
+            if( thrownSoftAbort ) return;
 
             thrownSoftAbort = true;
             throw new LuaError( TimeoutState.ABORT_MESSAGE );
         }
+
+        private void handlePause( DebugState ds, DebugFrame di ) throws LuaError, UnwindThrowable
+        {
+            // Preserve the current state
+            isPaused = true;
+            oldInHook = ds.inhook;
+            oldFlags = di.flags;
+
+            // Suspend the state. This will probably throw, but we need to handle the case where it won't.
+            di.flags |= FLAG_HOOKYIELD | FLAG_HOOKED;
+            LuaThread.suspend( ds.getLuaState() );
+            resetPaused( ds, di );
+        }
     }
 
     private class CobaltLuaContext implements ILuaContext

src/main/java/dan200/computercraft/shared/peripheral/modem/wired/TileCable.java

@@ -24,6 +24,7 @@ import net.minecraft.entity.player.EntityPlayer;
 import net.minecraft.nbt.NBTTagCompound;
 import net.minecraft.util.EnumFacing;
 import net.minecraft.util.EnumHand;
+import net.minecraft.util.math.AxisAlignedBB;
 import net.minecraft.util.math.BlockPos;
 import net.minecraft.util.math.Vec3d;
 import net.minecraft.util.text.TextComponentTranslation;
@@ -444,4 +445,11 @@ public class TileCable extends TileGeneric implements IPeripheralTile
         IBlockState state = getBlockState();
         return BlockCable.getPeripheralType( state );
     }
+
+    @Nonnull
+    @Override
+    public AxisAlignedBB getRenderBoundingBox()
+    {
+        return Block.FULL_BLOCK_AABB.offset( getPos() );
+    }
 }