More/performance (#558)

* VgaBios: check log level in ctor Signed-off-by: Maximilien Noal <[email protected]> * UI: The render invalidation method is a static lambda Signed-off-by: Maximilien Noal <[email protected]> * Mouse: Use hardware to get ticks (faster and more resilient) Signed-off-by: Maximilien Noal <[email protected]> * EmulationLoop: documented ctor argument Signed-off-by: Maximilien Noal <[email protected]> * EmulationLoop: Log properties are PascalCased Signed-off-by: Maximilien Noal <[email protected]> * Corrected documentation of ctor args Signed-off-by: Maximilien Noal <[email protected]> * CPU: IOPortDispatcher is a non-nullable private member Signed-off-by: Maximilien Noal <[email protected]> * CPU: inlined temp variable Signed-off-by: Maximilien Noal <[email protected]> * CPU: registersNames dicts as static FrozenDicts Signed-off-by: Maximilien Noal <[email protected]> * refactor: More readonly fields Signed-off-by: Maximilien Noal <[email protected]> * CPU: _stringOpCodes is a FrozenSet Signed-off-by: Maximilien Noal <[email protected]> * CA1859: Use concrete type to improve performance Signed-off-by: Maximilien Noal <[email protected]> * CA1859: Use concrete type to improve performance Signed-off-by: Maximilien Noal <[email protected]> * ConvertUtils: Source-generated Regex Signed-off-by: Maximilien Noal <[email protected]> * MemoryUtils: Removed unused methods Signed-off-by: Maximilien Noal <[email protected]> * PerformanceMeasurer: Avg only on last 30 secs Signed-off-by: Maximilien Noal <[email protected]> * ByteModificationRecord: readonly record struct (instead of record) Signed-off-by: Maximilien Noal <[email protected]> * FunctionCall: readonly record struct (instead of record) Signed-off-by: Maximilien Noal <[email protected]> * FunctionReturn: readonly record struct (instead of record) Signed-off-by: Maximilien Noal <[email protected]> --------- Signed-off-by: Maximilien Noal <[email protected]>
OpenRakis · Jan 18, 2024 · 63cb809 · 63cb809
1 parent 5b7d7bf
commit 63cb809
Show file tree

Hide file tree

Showing 29 changed files with 141 additions and 178 deletions.
diff --git a/src/Spice86.Core/Backend/Audio/PortAudio/PortAudioPlayer.cs b/src/Spice86.Core/Backend/Audio/PortAudio/PortAudioPlayer.cs
@@ -7,7 +7,7 @@ namespace Spice86.Core.Backend.Audio.PortAudio;
 /// The audio rendering backend
 /// </summary>
 public sealed class PortAudioPlayer : AudioPlayer {
-    private readonly IAudioEngine _engine;
+    private readonly PortAudioEngine _engine;
     private readonly PortAudioLib _portAudioLib;
 
     public PortAudioPlayer(PortAudioLib portAudioLib, int framesPerBuffer, AudioFormat format, double? suggestedLatency = null) : base(format) {

diff --git a/src/Spice86.Core/Emulator/CPU/CPU.cs b/src/Spice86.Core/Emulator/CPU/CPU.cs
@@ -13,11 +13,12 @@
 using Spice86.Core.Emulator.IOPorts;
 using Spice86.Core.Emulator.Memory;
 using Spice86.Core.Emulator.VM;
-using Spice86.Logging;
 using Spice86.Shared.Emulator.Memory;
 using Spice86.Shared.Interfaces;
 using Spice86.Shared.Utils;
 
+using System.Collections.Frozen;
+
 /// <summary>
 /// Implementation of a 8086 CPU. <br /> It has some 80186, 80286 and 80386 instructions as some
 /// program use them. <br /> It also has some x87 FPU instructions to support telling the programs
@@ -33,8 +34,8 @@ public class Cpu : IDebuggableComponent {
 
     private readonly ILoggerService _loggerService;
 
-    private static readonly HashSet<int> _stringOpCodes = new()
-        { 0xA4, 0xA5, 0xA6, 0xA7, 0xAA, 0xAB, 0xAC, 0xAD, 0xAE, 0xAF, 0x6C, 0x6D, 0x6E, 0x6F };
+    private static readonly FrozenSet<int> _stringOpCodes = new HashSet<int>()
+        { 0xA4, 0xA5, 0xA6, 0xA7, 0xAA, 0xAB, 0xAC, 0xAD, 0xAE, 0xAF, 0x6C, 0x6D, 0x6E, 0x6F }.ToFrozenSet();
 
     private readonly IMemory _memory;
 
@@ -57,17 +58,25 @@ public class Cpu : IDebuggableComponent {
     public int AddressSize { get; private set; }
 
 
-    // When true will crash if an interrupt targets code at 0000:0000
+    /// <summary>
+    /// When true will crash if an interrupt targets code at 0000:0000
+    /// </summary>
     public bool ErrorOnUninitializedInterruptHandler { get; set; } = true;
 
-    // interrupt not generated by the code
+    /// <summary>
+    /// interrupt not generated by the machine code
+    /// </summary>
     public byte? ExternalInterruptVectorNumber { get; private set; }
 
-    // Value used to read parts of the instruction.
-    // CPU uses this internally and adjusts IP after instruction execution is done.
+    /// <summary>
+    /// Value used to read parts of the instruction.
+    /// </summary>
+    /// <remarks>
+    /// CPU uses this internally and adjusts IP after instruction execution is done.
+    /// </remarks>
     private ushort _internalIp;
 
-    public IOPortDispatcher? IoPortDispatcher { get; set; }
+    private readonly IOPortDispatcher _ioPortDispatcher;
 
     public ExecutionFlowRecorder ExecutionFlowRecorder { get; }
 
@@ -78,7 +87,7 @@ public Cpu(IMemory memory, State state, DualPic dualPic, IOPortDispatcher ioPort
         _memory = memory;
         State = state;
         DualPic = dualPic;
-        IoPortDispatcher = ioPortDispatcher;
+        _ioPortDispatcher = ioPortDispatcher;
         _callbackHandler = callbackHandler;
         MachineBreakpoints = machineBreakpoints;
         InterruptVectorTable = new(_memory);
@@ -1074,11 +1083,10 @@ public void Aam(byte v2) {
     }
 
     public void Aad(byte v2) {
-        byte result = (byte)(State.AL + (State.AH * v2));
-        State.AL = result;
+        State.AL = (byte)(State.AL + (State.AH * v2));
         State.AH = 0;
         State.Flags.FlagRegister = 0;
-        _alu8.UpdateFlags(result);
+        _alu8.UpdateFlags(State.AL);
     }
 
     public void Aas() {
@@ -1326,31 +1334,22 @@ private void NearCall(ushort returnIP, ushort callIP) {
     }
 
     public byte In8(int port) {
-        if (IoPortDispatcher != null) {
-            return IoPortDispatcher.ReadByte((ushort)port);
-        }
-        return 0;
+        return _ioPortDispatcher.ReadByte((ushort)port);
     }
 
     public ushort In16(int port) {
-        if (IoPortDispatcher != null) {
-            return IoPortDispatcher.ReadWord((ushort)port);
-        }
-        return 0;
+        return _ioPortDispatcher.ReadWord((ushort)port);
     }
 
     public uint In32(int port) {
-        if (IoPortDispatcher != null) {
-            return IoPortDispatcher.ReadDWord((ushort)port);
-        }
-        return 0;
+        return _ioPortDispatcher.ReadDWord((ushort)port);
     }
 
-    public void Out8(int port, byte val) => IoPortDispatcher?.WriteByte((ushort)port, val);
+    public void Out8(int port, byte val) => _ioPortDispatcher.WriteByte((ushort)port, val);
 
-    public void Out16(int port, ushort val) => IoPortDispatcher?.WriteWord((ushort)port, val);
+    public void Out16(int port, ushort val) => _ioPortDispatcher.WriteWord((ushort)port, val);
 
-    public void Out32(int port, uint val) => IoPortDispatcher?.WriteDWord((ushort)port, val);
+    public void Out32(int port, uint val) => _ioPortDispatcher.WriteDWord((ushort)port, val);
 
     private bool ProcessPrefix(int opcode) {
         switch (opcode) {

diff --git a/src/Spice86.Core/Emulator/CPU/Registers/GeneralRegisters.cs b/src/Spice86.Core/Emulator/CPU/Registers/GeneralRegisters.cs
@@ -1,5 +1,7 @@
 namespace Spice86.Core.Emulator.CPU.Registers;
 
+using System.Collections.Frozen;
+
 /// <summary>
 /// Represents the x86 registers.
 /// </summary>
@@ -50,8 +52,9 @@ public class GeneralRegisters : RegistersHolder {
     public GeneralRegisters() : base(GetRegistersNames()) {
     }
 
-    private static Dictionary<uint, string> GetRegistersNames() {
-        return new() {
+    private static readonly FrozenDictionary<uint, string> _registersNames = new Dictionary<uint, string>()
+        {
+
             { AxIndex, "AX" },
             { CxIndex, "CX" },
             { DxIndex, "DX" },
@@ -60,6 +63,7 @@ private static Dictionary<uint, string> GetRegistersNames() {
             { BpIndex, "BP" },
             { SiIndex, "SI" },
             { DiIndex, "DI" }
-        };
-    }
+        }.ToFrozenDictionary();
+
+    private static FrozenDictionary<uint, string> GetRegistersNames() => _registersNames;
 }
diff --git a/src/Spice86.Core/Emulator/CPU/Registers/RegistersHolder.cs b/src/Spice86.Core/Emulator/CPU/Registers/RegistersHolder.cs
@@ -2,19 +2,21 @@
 
 using Spice86.Core.Emulator.Memory.ReaderWriter;
 
+using System.Collections.Frozen;
+
 /// <summary>
 /// A base class that represents a set of CPU registers.
 /// </summary>
 public class RegistersHolder {
     private readonly uint[] _registers;
 
-    private readonly Dictionary<uint, string> _registersNames;
+    private readonly FrozenDictionary<uint, string> _registersNames;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="RegistersHolder"/> class with the specified register names.
     /// </summary>
     /// <param name="registersNames">The names of the registers.</param>
-    protected RegistersHolder(Dictionary<uint, string> registersNames) {
+    protected RegistersHolder(FrozenDictionary<uint, string> registersNames) {
         _registersNames = registersNames;
         _registers = new uint[registersNames.Count];
         IUIntReaderWriter readerWriter = new UIntArrayReaderWriter(_registers);

diff --git a/src/Spice86.Core/Emulator/CPU/Registers/SegmentRegisters.cs b/src/Spice86.Core/Emulator/CPU/Registers/SegmentRegisters.cs
@@ -1,5 +1,6 @@
 namespace Spice86.Core.Emulator.CPU.Registers;
 
+using System.Collections.Frozen;
 using System.Collections.Generic;
 
 /// <summary>
@@ -43,18 +44,18 @@ public class SegmentRegisters : RegistersHolder {
     public SegmentRegisters() : base(GetRegistersNames()) {
     }
 
-    /// <summary>
-    /// Gets a dictionary that maps the segment register index to its name.
-    /// </summary>
-    /// <returns>A dictionary that maps the segment register index to its name.</returns>
-    private static Dictionary<uint, string> GetRegistersNames() {
-        return new() {
+    private static readonly FrozenDictionary<uint, string> _registersNames = new Dictionary<uint, string>() {
             { EsIndex, "ES" },
             { CsIndex, "CS" },
             { SsIndex, "SS" },
             { DsIndex, "DS" },
             { FsIndex, "FS" },
             { GsIndex, "GS" }
-        };
-    }
+        }.ToFrozenDictionary();
+
+    /// <summary>
+    /// Gets a dictionary that maps the segment register index to its name.
+    /// </summary>
+    /// <returns>A dictionary that maps the segment register index to its name.</returns>
+    private static FrozenDictionary<uint, string> GetRegistersNames() => _registersNames;
 }
diff --git a/src/Spice86.Core/Emulator/Devices/ExternalInput/DualPic.cs b/src/Spice86.Core/Emulator/Devices/ExternalInput/DualPic.cs
@@ -28,8 +28,8 @@ public class DualPic : DefaultIOPortHandler {
 
     private const byte BaseInterruptVectorSlave = 0x70;
 
-    private readonly IHardwareInterruptController _pic1;
-    private readonly IHardwareInterruptController _pic2;
+    private readonly Pic _pic1;
+    private readonly Pic _pic2;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="DualPic"/> class.

diff --git a/src/Spice86.Core/Emulator/Devices/Input/Mouse/Mouse.cs b/src/Spice86.Core/Emulator/Devices/Input/Mouse/Mouse.cs
@@ -6,8 +6,6 @@ namespace Spice86.Core.Emulator.Devices.Input.Mouse;
 using Spice86.Core.Emulator.Devices.ExternalInput;
 using Spice86.Core.Emulator.InterruptHandlers.Input.Mouse;
 using Spice86.Core.Emulator.IOPorts;
-using Spice86.Core.Emulator.Memory;
-using Spice86.Core.Emulator.VM;
 using Spice86.Shared.Emulator.Mouse;
 using Spice86.Shared.Interfaces;
 
@@ -137,7 +135,7 @@ private void OnMouseClick(object? sender, MouseButtonEventArgs eventArgs) {
     }
 
     private void UpdateMouse() {
-        long timestamp = DateTime.Now.Ticks;
+        long timestamp = System.Diagnostics.Stopwatch.GetTimestamp();
         // Check sample rate to see if we need to send an update yet.
         long ticksElapsed = timestamp - _lastUpdateTimestamp;
         if (ticksElapsed < _sampleRateTicks) {

diff --git a/src/Spice86.Core/Emulator/Function/ByteModificationRecord.cs b/src/Spice86.Core/Emulator/Function/ByteModificationRecord.cs
@@ -5,4 +5,4 @@
 /// </summary>
 /// <param name="OldValue">The old byte value.</param>
 /// <param name="NewValue">The new byte value.</param>
-public record ByteModificationRecord(byte OldValue, byte NewValue);
+public readonly record struct ByteModificationRecord(byte OldValue, byte NewValue);
diff --git a/src/Spice86.Core/Emulator/Function/Dump/GhidraSymbolsDumper.cs b/src/Spice86.Core/Emulator/Function/Dump/GhidraSymbolsDumper.cs
@@ -33,14 +33,14 @@ public void Dump(ExecutionFlowRecorder executionFlowRecorder, FunctionHandler fu
         ICollection<FunctionInformation> functionInformationsValues = functionHandler.FunctionInformations.Values;
         List<string> lines = new();
         // keep addresses in a set in order not to write a label where a function was, ghidra will otherwise overwrite functions with labels and this is not cool.
-        ISet<SegmentedAddress> dumpedAddresses = new HashSet<SegmentedAddress>();
+        HashSet<SegmentedAddress> dumpedAddresses = new HashSet<SegmentedAddress>();
         DumpFunctionInformations(lines, dumpedAddresses, functionInformationsValues);
         DumpLabels(lines, dumpedAddresses, executionFlowRecorder);
         using StreamWriter printWriter = new StreamWriter(destinationFilePath);
         lines.ForEach(line => printWriter.WriteLine(line));
     }
 
-    private void DumpLabels(List<string> lines, ISet<SegmentedAddress> dumpedAddresses, ExecutionFlowRecorder executionFlowRecorder) {
+    private void DumpLabels(List<string> lines, HashSet<SegmentedAddress> dumpedAddresses, ExecutionFlowRecorder executionFlowRecorder) {
         executionFlowRecorder.JumpsFromTo
             .SelectMany(x => x.Value)
             .Where(address => !dumpedAddresses.Contains(address))
@@ -55,15 +55,15 @@ private string DumpLabel(SegmentedAddress address) {
         return ToGhidraSymbol("spice86_label", address, "l");
     }
 
-    private void DumpFunctionInformations(ICollection<string> lines, ISet<SegmentedAddress> dumpedAddresses, ICollection<FunctionInformation> functionInformations) {
+    private void DumpFunctionInformations(List<string> lines, HashSet<SegmentedAddress> dumpedAddresses, ICollection<FunctionInformation> functionInformations) {
         functionInformations
             .OrderBy(functionInformation => functionInformation.Address)
             .Select(functionInformation => DumpFunctionInformation(dumpedAddresses, functionInformation))
             .ToList()
             .ForEach(line => lines.Add(line));
     }
 
-    private string DumpFunctionInformation(ISet<SegmentedAddress> dumpedAddresses, FunctionInformation functionInformation) {
+    private string DumpFunctionInformation(HashSet<SegmentedAddress> dumpedAddresses, FunctionInformation functionInformation) {
         dumpedAddresses.Add(functionInformation.Address);
         return ToGhidraSymbol(functionInformation.Name, functionInformation.Address, "f");
     }

diff --git a/src/Spice86.Core/Emulator/Function/ExecutionFlowRecorder.cs b/src/Spice86.Core/Emulator/Function/ExecutionFlowRecorder.cs
@@ -22,33 +22,33 @@ public class ExecutionFlowRecorder {
     /// <summary>
     /// Gets a dictionary of calls from one address to another.
     /// </summary>
-    public IDictionary<uint, ISet<SegmentedAddress>> CallsFromTo { get; set; }
-    private readonly ISet<ulong> _callsEncountered = new HashSet<ulong>(200000);
+    public IDictionary<uint, HashSet<SegmentedAddress>> CallsFromTo { get; set; }
+    private readonly HashSet<ulong> _callsEncountered = new(200000);
 
     /// <summary>
     /// Gets a dictionary of jumps from one address to another.
     /// </summary>
-    public IDictionary<uint, ISet<SegmentedAddress>> JumpsFromTo { get; set; }
-    private readonly ISet<ulong> _jumpsEncountered = new HashSet<ulong>(200000);
+    public IDictionary<uint, HashSet<SegmentedAddress>> JumpsFromTo { get; set; }
+    private readonly HashSet<ulong> _jumpsEncountered = new(200000);
 
     /// <summary>
     /// Gets a dictionary of returns from one address to another.
     /// </summary>
-    public IDictionary<uint, ISet<SegmentedAddress>> RetsFromTo { get; set; }
-    private readonly ISet<ulong> _retsEncountered = new HashSet<ulong>(200000);
+    public IDictionary<uint, HashSet<SegmentedAddress>> RetsFromTo { get; set; }
+    private readonly HashSet<ulong> _retsEncountered = new(200000);
 
     /// <summary>
     /// Gets a dictionary of unaligned returns from one address to another.
     /// </summary>
-    public IDictionary<uint, ISet<SegmentedAddress>> UnalignedRetsFromTo { get; set; }
-    private readonly ISet<ulong> _unalignedRetsEncountered = new HashSet<ulong>(200000);
+    public IDictionary<uint, HashSet<SegmentedAddress>> UnalignedRetsFromTo { get; set; }
+    private readonly HashSet<ulong> _unalignedRetsEncountered = new(200000);
 
     /// <summary>
     /// Gets the set of executed instructions.
     /// </summary>
-    public ISet<SegmentedAddress> ExecutedInstructions { get; set; }
-    private readonly ISet<uint> _instructionsEncountered = new HashSet<uint>(200000);
-    private readonly ISet<uint> _executableCodeAreasEncountered = new HashSet<uint>(200000);
+    public HashSet<SegmentedAddress> ExecutedInstructions { get; set; }
+    private readonly HashSet<uint> _instructionsEncountered = new(200000);
+    private readonly HashSet<uint> _executableCodeAreasEncountered = new(200000);
 
     /// <summary>
     /// Gets or sets whether we register self modifying machine code.
@@ -61,19 +61,19 @@ public class ExecutionFlowRecorder {
     /// The key of the outer dictionary is the modified byte address.
     /// The value of the outer dictionary is a dictionary of modifying instructions, where the key is the instruction address and the value is a set of possible changes that the instruction did.
     /// </summary>
-    public IDictionary<uint, IDictionary<uint, ISet<ByteModificationRecord>>> ExecutableAddressWrittenBy { get; }
+    public IDictionary<uint, IDictionary<uint, HashSet<ByteModificationRecord>>> ExecutableAddressWrittenBy { get; }
 
     /// <summary>
     /// Initializes a new instance. <see cref="RecordData"/> is set to false.
     /// </summary>
     public ExecutionFlowRecorder() {
         RecordData = false;
-        CallsFromTo = new Dictionary<uint, ISet<SegmentedAddress>>(200000);
-        JumpsFromTo = new Dictionary<uint, ISet<SegmentedAddress>>(200000);
-        RetsFromTo = new Dictionary<uint, ISet<SegmentedAddress>>(200000);
-        UnalignedRetsFromTo = new Dictionary<uint, ISet<SegmentedAddress>>(200000);
+        CallsFromTo = new Dictionary<uint, HashSet<SegmentedAddress>>(200000);
+        JumpsFromTo = new Dictionary<uint, HashSet<SegmentedAddress>>(200000);
+        RetsFromTo = new Dictionary<uint, HashSet<SegmentedAddress>>(200000);
+        UnalignedRetsFromTo = new Dictionary<uint, HashSet<SegmentedAddress>>(200000);
         ExecutedInstructions = new HashSet<SegmentedAddress>();
-        ExecutableAddressWrittenBy = new Dictionary<uint, IDictionary<uint, ISet<ByteModificationRecord>>>(200000);
+        ExecutableAddressWrittenBy = new Dictionary<uint, IDictionary<uint, HashSet<ByteModificationRecord>>>(200000);
     }
 
     /// <summary>
@@ -140,7 +140,7 @@ public void RegisterExecutedInstruction(ushort cs, ushort ip) {
     /// <param name="segment">The address segment.</param>
     /// <param name="offset">The address offset.</param>
     /// <returns><c>true</c> when the address was added, <c>false</c> if it was already there</returns>
-    private static bool AddSegmentedAddressInCache(ISet<uint> cache, ushort segment, ushort offset) {
+    private static bool AddSegmentedAddressInCache(HashSet<uint> cache, ushort segment, ushort offset) {
         return cache.Add(MemoryUtils.ToPhysicalAddress(segment, offset));
     }
 
@@ -207,18 +207,18 @@ private void RegisterExecutableByteModification(SegmentedAddress instructionAddr
             return;
         }
         if (!ExecutableAddressWrittenBy.TryGetValue(modifiedAddress,
-                out IDictionary<uint, ISet<ByteModificationRecord>>? instructionsChangingThisAddress)) {
-            instructionsChangingThisAddress = new Dictionary<uint, ISet<ByteModificationRecord>>();
+                out IDictionary<uint, HashSet<ByteModificationRecord>>? instructionsChangingThisAddress)) {
+            instructionsChangingThisAddress = new Dictionary<uint, HashSet<ByteModificationRecord>>();
             ExecutableAddressWrittenBy[modifiedAddress] = instructionsChangingThisAddress;
         }
-        if (!instructionsChangingThisAddress.TryGetValue(instructionAddressPhysical, out ISet<ByteModificationRecord>? byteModificationRecords)) {
+        if (!instructionsChangingThisAddress.TryGetValue(instructionAddressPhysical, out HashSet<ByteModificationRecord>? byteModificationRecords)) {
             byteModificationRecords = new HashSet<ByteModificationRecord>();
             instructionsChangingThisAddress[instructionAddressPhysical] = byteModificationRecords;
         }
         byteModificationRecords.Add(new ByteModificationRecord(oldValue, newValue));
     }
 
-    private void RegisterAddressJump(IDictionary<uint, ISet<SegmentedAddress>> FromTo, ISet<ulong> encountered, ushort fromCS, ushort fromIP, ushort toCS, ushort toIP) {
+    private void RegisterAddressJump(IDictionary<uint, HashSet<SegmentedAddress>> FromTo, HashSet<ulong> encountered, ushort fromCS, ushort fromIP, ushort toCS, ushort toIP) {
         if (!RecordData) {
             return;
         }
@@ -228,7 +228,7 @@ private void RegisterAddressJump(IDictionary<uint, ISet<SegmentedAddress>> FromT
         }
         encountered.Add(key);
         uint physicalFrom = MemoryUtils.ToPhysicalAddress(fromCS, fromIP);
-        if (!FromTo.TryGetValue(physicalFrom, out ISet<SegmentedAddress>? destinationAddresses)) {
+        if (!FromTo.TryGetValue(physicalFrom, out HashSet<SegmentedAddress>? destinationAddresses)) {
             destinationAddresses = new HashSet<SegmentedAddress>();
             FromTo.Add(physicalFrom, destinationAddresses);
         }