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.

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

Fixes yield proc scheduling. Turns sleeps into an opcode #1539

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
58 changes: 58 additions & 0 deletions Content.Tests/DMProject/Tests/Sleeping/YieldOrder.dm
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
var/counter
#define ExpectOrder(n) ASSERT(++counter == ##n)

/proc/BackgroundSleep(delay, expect)
set waitfor = FALSE
sleep(delay)
world.log << "Expect: [expect]"
ExpectOrder(expect)

#define MODE_INLINE 0 // spawn
#define MODE_BACKGROUND 1 // set waitfor = FALSE + sleep
#define MODE_RAND 2 // random seeded

#define TestSleep(delay, expect) if(mode == MODE_INLINE || (mode == MODE_RAND && prob(50))){ spawn(##delay) { ExpectOrder(##expect); } } else { BackgroundSleep(##delay, ##expect); }

/proc/TestSequence(mode)
counter = 0
var/start_tick = world.time

TestSleep(0, 2)
ExpectOrder(1)
sleep(0)
ExpectOrder(3)

TestSleep(-1, 4)
ExpectOrder(5)

TestSleep(0, 6)
sleep(-1)
ExpectOrder(7)

TestSleep(-1, 8)
ExpectOrder(9)
sleep(-1)
ExpectOrder(10)

TestSleep(1, 13)
sleep(-1)
ExpectOrder(11)
sleep(0)
ExpectOrder(12)

ASSERT(world.time == start_tick)

sleep(1)
ExpectOrder(14)

/proc/RunTest()
world.log << "Inline:"
TestSequence(MODE_INLINE)

world.log << "Background:"
TestSequence(MODE_BACKGROUND)

rand_seed(22475)
for(var/i in 1 to 10000)
world.log << "Rand-[i]:"
TestSequence(MODE_RAND)
Comment on lines +55 to +58
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to up the test timeout because this overran 500ms

6 changes: 4 additions & 2 deletions Content.Tests/DMTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using NUnit.Framework;
using OpenDreamRuntime;
using OpenDreamRuntime.Objects;
using OpenDreamRuntime.Procs;
using OpenDreamRuntime.Rendering;
using OpenDreamShared.Rendering;
using Robust.Shared.Asynchronous;
Expand All @@ -28,6 +29,7 @@ public sealed class DMTests : ContentUnitTest {

[Dependency] private readonly DreamManager _dreamMan = default!;
[Dependency] private readonly DreamObjectTree _objectTree = default!;
[Dependency] private readonly ProcScheduler _procScheduler = default!;
[Dependency] private readonly ITaskManager _taskManager = default!;

[Flags]
Expand Down Expand Up @@ -138,11 +140,11 @@ public void TestFiles(string sourceFile, DMTestFlags testFlags) {
watch.Start();

// Tick until our inner call has finished
while (!callTask.IsCompleted) {
while (!callTask.IsCompleted || _procScheduler.HasProcsQueued || _procScheduler.HasProcsSleeping) {
_dreamMan.Update();
_taskManager.ProcessPendingTasks();

if (watch.Elapsed.TotalMilliseconds > 500) {
if (watch.Elapsed.TotalSeconds > 5) {
Assert.Fail("Test timed out");
}
}
Expand Down
2 changes: 2 additions & 0 deletions DMCompiler/Bytecode/DreamProcOpcode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ public enum DreamProcOpcode : byte {
Log = 0x81,
LogE = 0x82,
Abs = 0x83,
[OpcodeMetadata(stackDelta: -1)] Sleep = 0x84,
BackgroundSleep = 0x85,
}

/// <summary>
Expand Down
17 changes: 17 additions & 0 deletions DMCompiler/Compiler/DM/DMAST.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ public void VisitProcStatementSpawn(DMASTProcStatementSpawn statementSpawn) {
throw new NotImplementedException();
}

public void VisitProcStatementSleep(DMASTProcStatementSleep statementSleep) {
throw new NotImplementedException();
}

public void VisitProcStatementIf(DMASTProcStatementIf statementIf) {
throw new NotImplementedException();
}
Expand Down Expand Up @@ -920,6 +924,19 @@ public override void Visit(DMASTVisitor visitor) {
}
}

public sealed class DMASTProcStatementSleep : DMASTProcStatement {
public DMASTExpression Delay;

public DMASTProcStatementSleep(Location location, DMASTExpression delay) :
base(location) {
Delay = delay;
}

public override void Visit(DMASTVisitor visitor) {
visitor.VisitProcStatementSleep(this);
}
}

public sealed class DMASTProcStatementSpawn : DMASTProcStatement {
public DMASTExpression Delay;
public readonly DMASTProcBlockInner Body;
Expand Down
1 change: 1 addition & 0 deletions DMCompiler/Compiler/DM/DMLexer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public sealed class DMLexer : TokenLexer {
{ "call", TokenType.DM_Call },
{ "call_ext", TokenType.DM_Call},
{ "spawn", TokenType.DM_Spawn },
{ "sleep", TokenType.DM_Sleep },
{ "goto", TokenType.DM_Goto },
{ "step", TokenType.DM_Step },
{ "try", TokenType.DM_Try },
Expand Down
19 changes: 19 additions & 0 deletions DMCompiler/Compiler/DM/DMParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ public DMParser(DMLexer lexer) : base(lexer) {
TokenType.DM_Null,
TokenType.DM_Switch,
TokenType.DM_Spawn,
TokenType.DM_Sleep,
TokenType.DM_Do,
TokenType.DM_While,
TokenType.DM_For,
Expand Down Expand Up @@ -715,6 +716,7 @@ public DMASTFile File() {
procStatement ??= Switch();
procStatement ??= Continue();
procStatement ??= Break();
procStatement ??= Sleep();
procStatement ??= Spawn();
procStatement ??= While();
procStatement ??= DoWhile();
Expand Down Expand Up @@ -1059,6 +1061,23 @@ private DMASTProcStatementSet[] ProcSetEnd(bool allowMultiple) {
}
}

public DMASTProcStatementSleep? Sleep() {
var loc = Current().Location;

if (Check(TokenType.DM_Sleep)) {
Whitespace();
bool hasParenthesis = Check(TokenType.DM_LeftParenthesis);
Whitespace();
DMASTExpression? delay = Expression();
if (delay == null) Error("Expected delay to sleep for");
if (hasParenthesis) ConsumeRightParenthesis();

return new DMASTProcStatementSleep(loc, delay ?? new DMASTConstantInteger(loc, 0));
} else {
return null;
}
}

public DMASTProcStatementIf? If() {
var loc = Current().Location;

Expand Down
21 changes: 10 additions & 11 deletions DMCompiler/DM/DMProc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -459,20 +459,19 @@ public void MarkLoopContinue(string loopLabel) {
AddLabel($"{loopLabel}_continue");
}

public void BackgroundSleep() {
// TODO This seems like a bad way to handle background, doesn't it?

if ((Attributes & ProcAttributes.Background) == ProcAttributes.Background) {
if (!DMObjectTree.TryGetGlobalProc("sleep", out var sleepProc)) {
throw new CompileErrorException(Location, "Cannot do a background sleep without a sleep proc");
}

PushFloat(-1); // argument given to sleep()
Call(DMReference.CreateGlobalProc(sleepProc.Id), DMCallArgumentsType.FromStack, 1);
Pop(); // Pop the result of the sleep call
public void SleepDelayPushed() => WriteOpcode(DreamProcOpcode.Sleep);

public void Sleep(float delay) {
if(delay == -1.0f) // yielding
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be <= 0

WriteOpcode(DreamProcOpcode.BackgroundSleep);
else {
PushFloat(delay);
WriteOpcode(DreamProcOpcode.Sleep);
}
}

public void BackgroundSleep() => WriteOpcode(DreamProcOpcode.BackgroundSleep);

public void LoopJumpToStart(string loopLabel) {
BackgroundSleep();
Jump($"{loopLabel}_start");
Expand Down
4 changes: 4 additions & 0 deletions DMCompiler/DM/Visitors/DMASTSimplifier.cs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ public void VisitProcStatementSpawn(DMASTProcStatementSpawn statementSpawn) {
statementSpawn.Body.Visit(this);
}

public void VisitProcStatementSleep(DMASTProcStatementSleep statementSleep) {
SimplifyExpression(ref statementSleep.Delay);
}

public void VisitProcStatementGoto(DMASTProcStatementGoto statementGoto) {

}
Expand Down
16 changes: 16 additions & 0 deletions DMCompiler/DM/Visitors/DMProcBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ public void ProcessStatement(DMASTProcStatement statement) {
case DMASTProcStatementBreak statementBreak: ProcessStatementBreak(statementBreak); break;
case DMASTProcStatementDel statementDel: ProcessStatementDel(statementDel); break;
case DMASTProcStatementSpawn statementSpawn: ProcessStatementSpawn(statementSpawn); break;
case DMASTProcStatementSleep statementSleep: ProcessStatementSleep(statementSleep); break;
case DMASTProcStatementReturn statementReturn: ProcessStatementReturn(statementReturn); break;
case DMASTProcStatementIf statementIf: ProcessStatementIf(statementIf); break;
case DMASTProcStatementFor statementFor: ProcessStatementFor(statementFor); break;
Expand Down Expand Up @@ -317,6 +318,21 @@ public void ProcessStatementSpawn(DMASTProcStatementSpawn statementSpawn) {
_proc.AddLabel(afterSpawnLabel);
}

public void ProcessStatementSleep(DMASTProcStatementSleep statementSleep) {
var expr = DMExpression.Create(_dmObject, _proc, statementSleep.Delay);
if (expr.TryAsConstant(out var constant)) {
if (constant is Number constantNumber) {
_proc.Sleep(constantNumber.Value);
return;
}

constant.EmitPushValue(_dmObject, _proc);
} else
expr.EmitPushValue(_dmObject, _proc);

_proc.SleepDelayPushed();
}

public void ProcessStatementVarDeclaration(DMASTProcStatementVarDeclaration varDeclaration) {
if (varDeclaration.IsGlobal) { return; } //Currently handled by DMObjectBuilder

Expand Down
1 change: 0 additions & 1 deletion DMCompiler/DMStandard/_Standard.dm
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ proc/roll(ndice = 1, sides)
proc/round(A, B)
proc/sha1(input)
proc/shutdown(Addr,Natural = 0)
proc/sleep(Delay)
proc/sorttext(T1, T2)
proc/sorttextEx(T1, T2)
proc/sound(file, repeat = 0, wait, channel, volume)
Expand Down
4 changes: 2 additions & 2 deletions OpenDreamRuntime/Procs/AsyncNativeProc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

namespace OpenDreamRuntime.Procs {
public sealed class AsyncNativeProc : DreamProc {
public sealed class State : ProcState {
public sealed class State : AsyncProcState {
public static readonly Stack<State> Pool = new();

// IoC dependencies instead of proc fields because _proc can be null
Expand Down Expand Up @@ -52,7 +52,7 @@ public void Initialize(AsyncNativeProc? proc, Func<State, Task<DreamValue>> task
}

// Used to avoid reentrant resumptions in our proc
public void SafeResume() {
public override void SafeResume() {
if (_inResume) {
return;
}
Expand Down
5 changes: 5 additions & 0 deletions OpenDreamRuntime/Procs/AsyncProcState.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
namespace OpenDreamRuntime.Procs {
public abstract class AsyncProcState : ProcState {
public abstract void SafeResume();
}
}
Loading
Loading