From 71ee692da014c4f49983da63705ac745a64f5129 Mon Sep 17 00:00:00 2001 From: SquidDev Date: Thu, 22 Nov 2018 11:50:35 +0000 Subject: [PATCH] Some more improvements to threading - Put all ComputerCraft threads into a thread group - Be a little more aggressive in when we cleanup/abandon threads. --- .../core/apis/http/HTTPExecutor.java | 10 +-- .../core/computer/ComputerThread.java | 20 ++---- .../core/lua/CobaltLuaMachine.java | 70 ++++++------------- .../shared/util/ThreadUtils.java | 68 ++++++++++++++++++ 4 files changed, 98 insertions(+), 70 deletions(-) create mode 100644 src/main/java/dan200/computercraft/shared/util/ThreadUtils.java diff --git a/src/main/java/dan200/computercraft/core/apis/http/HTTPExecutor.java b/src/main/java/dan200/computercraft/core/apis/http/HTTPExecutor.java index bd34b5090..d9e1bd3b8 100644 --- a/src/main/java/dan200/computercraft/core/apis/http/HTTPExecutor.java +++ b/src/main/java/dan200/computercraft/core/apis/http/HTTPExecutor.java @@ -8,7 +8,7 @@ package dan200.computercraft.core.apis.http; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; -import com.google.common.util.concurrent.ThreadFactoryBuilder; +import dan200.computercraft.shared.util.ThreadUtils; import io.netty.channel.EventLoopGroup; import io.netty.channel.nio.NioEventLoopGroup; @@ -25,17 +25,13 @@ public final class HTTPExecutor 4, Integer.MAX_VALUE, 60L, TimeUnit.SECONDS, new SynchronousQueue<>(), - new ThreadFactoryBuilder() - .setDaemon( true ) + ThreadUtils.builder( "HTTP" ) .setPriority( Thread.MIN_PRIORITY + (Thread.NORM_PRIORITY - Thread.MIN_PRIORITY) / 2 ) - .setNameFormat( "ComputerCraft-HTTP-%d" ) .build() ) ); - public static final EventLoopGroup LOOP_GROUP = new NioEventLoopGroup( 4, new ThreadFactoryBuilder() - .setDaemon( true ) + public static final EventLoopGroup LOOP_GROUP = new NioEventLoopGroup( 4, ThreadUtils.builder( "Netty" ) .setPriority( Thread.MIN_PRIORITY + (Thread.NORM_PRIORITY - Thread.MIN_PRIORITY) / 2 ) - .setNameFormat( "ComputerCraft-Netty-%d" ) .build() ); diff --git a/src/main/java/dan200/computercraft/core/computer/ComputerThread.java b/src/main/java/dan200/computercraft/core/computer/ComputerThread.java index 2f17c9749..a4e538ee0 100644 --- a/src/main/java/dan200/computercraft/core/computer/ComputerThread.java +++ b/src/main/java/dan200/computercraft/core/computer/ComputerThread.java @@ -8,13 +8,14 @@ package dan200.computercraft.core.computer; import dan200.computercraft.ComputerCraft; import dan200.computercraft.core.tracking.Tracking; +import dan200.computercraft.shared.util.ThreadUtils; import java.util.HashSet; import java.util.Set; import java.util.WeakHashMap; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.ThreadFactory; public class ComputerThread { @@ -56,8 +57,8 @@ public class ComputerThread */ private static Thread[] s_threads = null; - private static final AtomicInteger s_ManagerCounter = new AtomicInteger( 1 ); - private static final AtomicInteger s_DelegateCounter = new AtomicInteger( 1 ); + private static final ThreadFactory s_ManagerFactory = ThreadUtils.factory( "Computer-Manager" ); + private static final ThreadFactory s_RunnerFactory = ThreadUtils.factory( "Computer-Runner" ); /** * Start the computer thread @@ -72,16 +73,12 @@ public class ComputerThread s_threads = new Thread[ComputerCraft.computer_threads]; } - SecurityManager manager = System.getSecurityManager(); - final ThreadGroup group = manager == null ? Thread.currentThread().getThreadGroup() : manager.getThreadGroup(); for( int i = 0; i < s_threads.length; i++ ) { Thread thread = s_threads[i]; if( thread == null || !thread.isAlive() ) { - thread = s_threads[i] = new Thread( group, new TaskExecutor(), "ComputerCraft-Computer-Manager-" + s_ManagerCounter.getAndIncrement() ); - thread.setDaemon( true ); - thread.start(); + (s_threads[i] = s_ManagerFactory.newThread( new TaskExecutor() )).start(); } } } @@ -188,12 +185,7 @@ public class ComputerThread if( thread == null || !thread.isAlive() ) { runner = new TaskRunner(); - - SecurityManager manager = System.getSecurityManager(); - final ThreadGroup group = manager == null ? Thread.currentThread().getThreadGroup() : manager.getThreadGroup(); - Thread thread = this.thread = new Thread( group, runner, "ComputerCraft-Computer-Runner" + s_DelegateCounter.getAndIncrement() ); - thread.setDaemon( true ); - thread.start(); + (thread = s_RunnerFactory.newThread( runner )).start(); } long start = System.nanoTime(); diff --git a/src/main/java/dan200/computercraft/core/lua/CobaltLuaMachine.java b/src/main/java/dan200/computercraft/core/lua/CobaltLuaMachine.java index 0cb1f0c6d..e933397f5 100644 --- a/src/main/java/dan200/computercraft/core/lua/CobaltLuaMachine.java +++ b/src/main/java/dan200/computercraft/core/lua/CobaltLuaMachine.java @@ -6,7 +6,6 @@ package dan200.computercraft.core.lua; -import com.google.common.util.concurrent.ThreadFactoryBuilder; import dan200.computercraft.ComputerCraft; import dan200.computercraft.api.lua.*; import dan200.computercraft.core.computer.Computer; @@ -14,6 +13,7 @@ import dan200.computercraft.core.computer.ITask; import dan200.computercraft.core.computer.MainThread; import dan200.computercraft.core.tracking.Tracking; import dan200.computercraft.core.tracking.TrackingField; +import dan200.computercraft.shared.util.ThreadUtils; import org.squiddev.cobalt.*; import org.squiddev.cobalt.compiler.CompileException; import org.squiddev.cobalt.compiler.LoadState; @@ -48,18 +48,15 @@ public class CobaltLuaMachine implements ILuaMachine 0, Integer.MAX_VALUE, 60L, TimeUnit.SECONDS, new SynchronousQueue<>(), - new ThreadFactoryBuilder() - .setDaemon( true ) - .setNameFormat( "ComputerCraft-Coroutine-%d" ) - .build() + ThreadUtils.factory( "Coroutine" ) ); private final Computer m_computer; - private final LuaState m_state; - private final LuaTable m_globals; - + private LuaState m_state; + private LuaTable m_globals; private LuaThread m_mainRoutine; + private String m_eventFilter; private String m_softAbortMessage; private String m_hardAbortMessage; @@ -132,6 +129,7 @@ public class CobaltLuaMachine implements ILuaMachine Tracking.addValue( m_computer, TrackingField.COROUTINES_DISPOSED, 1 ); } } ); + ComputerCraft.log.info( "Thread pool: " + coroutines ); } ) .build(); @@ -189,10 +187,7 @@ public class CobaltLuaMachine implements ILuaMachine public void loadBios( InputStream bios ) { // Begin executing a file (ie, the bios) - if( m_mainRoutine != null ) - { - return; - } + if( m_mainRoutine != null ) return; try { @@ -201,30 +196,19 @@ public class CobaltLuaMachine implements ILuaMachine } catch( CompileException e ) { - if( m_mainRoutine != null ) - { - m_mainRoutine.abandon(); - m_mainRoutine = null; - } + unload(); } catch( IOException e ) { ComputerCraft.log.warn( "Could not load bios.lua ", e ); - if( m_mainRoutine != null ) - { - m_mainRoutine.abandon(); - m_mainRoutine = null; - } + unload(); } } @Override public void handleEvent( String eventName, Object[] arguments ) { - if( m_mainRoutine == null ) - { - return; - } + if( m_mainRoutine == null ) return; if( m_eventFilter != null && eventName != null && !eventName.equals( m_eventFilter ) && !eventName.equals( "terminate" ) ) { @@ -251,26 +235,14 @@ public class CobaltLuaMachine implements ILuaMachine else { LuaValue filter = results.arg( 2 ); - if( filter.isString() ) - { - m_eventFilter = filter.toString(); - } - else - { - m_eventFilter = null; - } + m_eventFilter = filter.isString() ? filter.toString() : null; } - LuaThread mainThread = m_mainRoutine; - if( mainThread.getStatus().equals( "dead" ) ) - { - m_mainRoutine = null; - } + if( m_mainRoutine.getStatus().equals( "dead" ) ) unload(); } catch( LuaError e ) { - m_mainRoutine.abandon(); - m_mainRoutine = null; + unload(); } finally { @@ -307,18 +279,18 @@ public class CobaltLuaMachine implements ILuaMachine @Override public boolean isFinished() { - return (m_mainRoutine == null); + return m_mainRoutine == null; } @Override public void unload() { - if( m_mainRoutine != null ) - { - LuaThread mainThread = m_mainRoutine; - mainThread.abandon(); - m_mainRoutine = null; - } + if( m_state == null ) return; + + m_state.abandon(); + m_mainRoutine = null; + m_state = null; + m_globals = null; } private LuaTable wrapLuaObject( ILuaObject object ) @@ -714,7 +686,7 @@ public class CobaltLuaMachine implements ILuaMachine private byte[] bytes; private int offset, remaining = 0; - public StringInputStream( LuaState state, LuaValue func ) + StringInputStream( LuaState state, LuaValue func ) { this.state = state; this.func = func; diff --git a/src/main/java/dan200/computercraft/shared/util/ThreadUtils.java b/src/main/java/dan200/computercraft/shared/util/ThreadUtils.java new file mode 100644 index 000000000..197a608a1 --- /dev/null +++ b/src/main/java/dan200/computercraft/shared/util/ThreadUtils.java @@ -0,0 +1,68 @@ +/* + * This file is part of ComputerCraft - http://www.computercraft.info + * Copyright Daniel Ratcliffe, 2011-2018. Do not distribute without permission. + * Send enquiries to dratcliffe@gmail.com + */ + +package dan200.computercraft.shared.util; + +import com.google.common.util.concurrent.ThreadFactoryBuilder; + +import java.util.concurrent.ThreadFactory; + +/** + * Provides some utilities to create thread groups + */ +public final class ThreadUtils +{ + private static final ThreadGroup baseGroup = new ThreadGroup( "ComputerCraft" ); + + private ThreadUtils() + { + } + + /** + * Construct a group under ComputerCraft's shared group + * + * @param name The group's name. This will be prefixed with "ComputerCraft-". + * @return The constructed thread group. + */ + public static ThreadGroup group( String name ) + { + return new ThreadGroup( baseGroup, baseGroup.getName() + "-" + name ); + } + + /** + * Create a new {@link ThreadFactoryBuilder}, which constructs threads under a group of the given {@code name}. + * + * Each thread will be of the format {@code ComputerCraft--}, and belong to a group + * called {@code ComputerCraft-} (which in turn will be a child group of the main {@code ComputerCraft} group. + * + * @param name The name for the thread group and child threads. + * @return The constructed thread factory builder, which may be extended with other properties. + * @see #factory(String) + */ + public static ThreadFactoryBuilder builder( String name ) + { + ThreadGroup group = group( name ); + return new ThreadFactoryBuilder() + .setDaemon( true ) + .setNameFormat( group.getName().replace( "%", "%%" ) + "-%d" ) + .setThreadFactory( x -> new Thread( group, x ) ); + } + + /** + * Create a new {@link ThreadFactory}, which constructs threads under a group of the given {@code name}. + * + * Each thread will be of the format {@code ComputerCraft--}, and belong to a group + * called {@code ComputerCraft-} (which in turn will be a child group of the main {@code ComputerCraft} group. + * + * @param name The name for the thread group and child threads. + * @return The constructed thread factory. + * @see #builder(String) + */ + public static ThreadFactory factory( String name ) + { + return builder( name ).build(); + } +}