refactor: Optimized & improved DosExecResult and DosFileOperationResult.

[assumenothing]

Description of Changes

• DosExecResult now implements IEquatable<T> interface and overrides Equals(object?) and GetHashCode().
• DosExecResult added static cache for storing a subset of common error results.
• DosFileOperationResult now implements IEquatable<T> interface and overrides Equals(object?) and GetHashCode().
• DosFileOperationResult added static cache for storing a subset of common error results, NoValue() result, and repeated Value16() results (with the same value).
• DosFileOperationResult class is now sealed (was not possible to inherit before with private constructor anyways).
• Added parameter names to a few calls to make it more obvious what parameter values are being passed into the constructors.

Rationale behind Changes

Minor performance improvements for fixed/common/repeated operation results (reduces heap allocations).

It would also be possible to easily cache the fixed result from DosExecResult.SuccessLoadOverlay(), but this is most likely not called often enough to need that optimization.

NOTE: It would be better/more efficient to change DosExecResult and DosFileOperationResult from class (reference type) to readonly struct (value type), as there would then be no need to apply these caching optimizations. (This would further reduce heap allocations, may slightly improve memory accesses as the results would generally be stored in the thread stack rather than the GC heap, would potentially avoid an extra pointer dereference, and would have the minor benefit of being able to safely implement the == and != operators). However, these are public APIs and changing the fundamental object types would be a breaking change.

Suggested Testing Steps

Tested against project unit tests.

[Copilot]

Pull request overview

This PR refactors DosExecResult and DosFileOperationResult to reduce allocations by introducing caches for common results and by adding value-based equality (IEquatable<T>, Equals, GetHashCode) so these result objects can be compared and hashed consistently.

Changes:

• Added static caching for common error/no-value/value-16 results to reduce repeated allocations.
• Implemented IEquatable<T> and overrides for Equals(object?) / GetHashCode() on both result types.
• Tightened API surface (DosFileOperationResult is now sealed) and made some constructor calls more explicit via named arguments.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
src/Spice86.Core/Emulator/OperatingSystem/Structures/DosFileOperationResult.cs	Adds result caching (errors/no-value/last Value16) and value-based equality/hashing.
src/Spice86.Core/Emulator/OperatingSystem/Structures/DosExecResult.cs	Adds error-result caching and value-based equality/hashing for EXEC results.

Comments suppressed due to low confidence (1)

src/Spice86.Core/Emulator/OperatingSystem/Structures/DosFileOperationResult.cs:107

• This new DosFileOperationResult(...) call mixes named and positional arguments (value: null, refCount), which is not valid C#. Name the last argument as well (e.g., refCount: refCount) or make all arguments positional.

    /// <param name="refCount">The reference count from the System File Table.</param>
    /// <returns>A new instance of the class with a reference count.</returns>
    public static DosFileOperationResult NoValueWithRefCount(byte refCount) {
        return new DosFileOperationResult(error: false, valueIsUint32: false, value: null, refCount);
    }

[assumenothing]

I did think about the possible thread safety issues (and fixed one of the theoretically possible threading issues I found in my first commit). But it shouldn't really matter whether the caches thread-safe or not: they are immutable objects and the static fields references are copied into local variables before accessing them. I could add the volatile keyword to the fields or use the Volatile class methods, but again it shouldn't really make much difference (some duplicate instances are better than a lot of duplicates like it was prior to these changes--these are small objects anyways).

[maximilien-noal]

@assumenothing This is a natural evolution of those classes, and yeah they should have been sealed long ago.

I'm always on the side of more performance, especially since long GC pauses are problematic with sound rendering.

Approved! :)