Skip to content

Commit

Permalink
Move zstd wrapping code to BizHawk.Common
Browse files Browse the repository at this point in the history 
thanks delegate*
  • Loading branch information
@CasualPokePlayer
CasualPokePlayer committed May 1, 2024
1 parent f0a2ea1 commit c2f549d
Show file tree
Hide file tree
Showing 10 changed files with 207 additions and 140 deletions.
30 changes: 15 additions & 15 deletions src/BizHawk.Client.Common/FrameworkZipWriter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
using System.IO;
using System.IO.Compression;

using BizHawk.Emulation.Common;
using BizHawk.Common;

namespace BizHawk.Client.Common
{
public class FrameworkZipWriter : IZipWriter
{
private ZipArchive _archive;
private ZipArchive _archive;
private Zstd _zstd;
private readonly CompressionLevel _level;
private readonly int _zstdCompressionLevel;
Expand All @@ -22,11 +22,11 @@ public FrameworkZipWriter(string path, int compressionLevel)
else if (compressionLevel < 5)
_level = CompressionLevel.Fastest;
else
_level = CompressionLevel.Optimal;

_zstd = new();
// compressionLevel ranges from 0 to 9
// normal compression level range for zstd is 1 to 19
_level = CompressionLevel.Optimal;

_zstd = new();
// compressionLevel ranges from 0 to 9
// normal compression level range for zstd is 1 to 19
_zstdCompressionLevel = compressionLevel * 2 + 1;
}

Expand All @@ -35,13 +35,13 @@ public void WriteItem(string name, Action<Stream> callback, bool zstdCompress)
using var stream = _archive.CreateEntry(name, _level).Open();

if (zstdCompress)
{
using var z = _zstd.CreateZstdCompressionStream(stream, _zstdCompressionLevel);
callback(z);
{
using var z = _zstd.CreateZstdCompressionStream(stream, _zstdCompressionLevel);
callback(z);
}
else
{
callback(stream);
{
callback(stream);
}
}

Expand All @@ -54,9 +54,9 @@ public void Dispose()
}

if (_zstd != null)
{
_zstd.Dispose();
_zstd = null;
{
_zstd.Dispose();
_zstd = null;
}
}
}
Expand Down
1 change: 0 additions & 1 deletion src/BizHawk.Client.Common/rewind/ZwinderBuffer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System.Linq;

using BizHawk.Common;
using BizHawk.Emulation.Common;

namespace BizHawk.Client.Common
{
Expand Down
24 changes: 12 additions & 12 deletions src/BizHawk.Client.Common/savestates/ZipStateLoader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
using System.IO.Compression;
using System.Linq;

using BizHawk.Emulation.Common;
using BizHawk.Common;

namespace BizHawk.Client.Common
{
Expand Down Expand Up @@ -91,13 +91,13 @@ public static ZipStateLoader LoadAndDetect(string filename, bool isMovieLoad = f
ret._zip = new ZipArchive(new FileStream(filename, FileMode.Open, FileAccess.Read), ZipArchiveMode.Read);
ret.PopulateEntries();
if (isMovieLoad)
{
if (!ret.GetLump(BinaryStateLump.ZipVersion, false, ret.ReadZipVersion, false))
{
// movies before 1.0.2 did not include the BizState 1.0 file, don't strictly error in this case
ret._ver = new Version(1, 0, 0);
Console.WriteLine("Read a zipstate of version {0}", ret._ver);
}
{
if (!ret.GetLump(BinaryStateLump.ZipVersion, false, ret.ReadZipVersion, false))
{
// movies before 1.0.2 did not include the BizState 1.0 file, don't strictly error in this case
ret._ver = new Version(1, 0, 0);
Console.WriteLine("Read a zipstate of version {0}", ret._ver);
}
}
else if (!ret.GetLump(BinaryStateLump.ZipVersion, false, ret.ReadZipVersion, false))
{
Expand All @@ -123,16 +123,16 @@ public bool GetLump(BinaryStateLump lump, bool abort, Action<Stream, long> callb
{
if (_entriesByName.TryGetValue(lump.ReadName, out var e))
{
using var zs = e.Open();

using var zs = e.Open();

if (isZstdCompressed && _ver.Build > 1)
{
using var z = _zstd.CreateZstdDecompressionStream(zs);
callback(z, e.Length);
}
else
{
callback(zs, e.Length);
{
callback(zs, e.Length);
}

return true;
Expand Down
131 changes: 131 additions & 0 deletions src/BizHawk.Common/zstd/LibZstd.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
using System;

This comment has been minimized.

Sign in to view

Copy link
@nattthebear

nattthebear May 18, 2024

Contributor

I liked the old one better, half as many LOC and no duplication of signatures.

Although, remind me why we can't use [DllImport] for these sorts of static things?

This comment has been minimized.

Sign in to view

Copy link
@YoshiRulz

YoshiRulz May 18, 2024

Member

Different filename on Linux.

This comment has been minimized.

Sign in to view

Copy link
@CasualPokePlayer

CasualPokePlayer May 18, 2024 •
edited
Loading

Author Member

Using delegate* would also be slightly faster if anything I believe (since it uses calli). And really, the LOC count could be heavily reduced by making it so this wasn't something BizInvoke did with Reflection / IL generation, but rather with a source generator using delegate* (and bonus, no IL generation means trimming and AOT compatible) so the extra LOC count is hidden in generated source files (hell, you could possibly remove all LOC and make a source generator directly parse a .txt/.json/.xml/whatever file describing everything about the API). THis is something I plan to do eventually to replace BizInvoke in general but it's taken a backseat with other things.

This comment has been minimized.

Sign in to view

Copy link
@nattthebear

nattthebear May 18, 2024

Contributor

BizInvoke uses calli: https://github.com/TASEmulators/BizHawk/blob/master/src/BizHawk.BizInvoke/BizInvoker.cs#L404.

I don't see the relevance of rewriting BizInvoker here? Either way, this is not using that anymore and so is stuck with this manual code.

This comment has been minimized.

Sign in to view

Copy link
@YoshiRulz

YoshiRulz May 18, 2024

Member

See also #3768; it would be nice if we could re-use the [LibraryImport] generator rather than write our own.

This comment has been minimized.

Sign in to view

Copy link
@CasualPokePlayer

CasualPokePlayer May 18, 2024 •
edited
Loading

Author Member

LibraryImport is not suitable as a replacement for BizInvoke in general due to the IMonitor and other requirements which disallow static instantiation, which is mainly relevant for waterboxing where it is primarily used.

The main reason I even moved Zstd handling over to BizHawk.Common is because of the dependency chain, as Zstd handling is now being used in Client.Common, Emulation.Cores, and Emulation.DiscSystem. We also want Zstd handling to be fast since it's handled in rather performance centric code, hence the usage of BizInvoke (and then the dll/so naming thing). There aren't a lot of good places where that can even be placed: BizInvoke usage can't be in Common, as BizInvoke depends on Common, not the other way around, and BizInvoke can't be placed as .NET Standard 2.0, so it is a a separate project instead of being in Common. Previously, the Zstd handling code was placed in Emulation.Common as it wasn't used in Emulation.DiscSystem. However, Emulation.Common depends on Emulation.DiscSystem, not the other way around, so Emulation.DiscSystem couldn't use it even if it wanted to. Putting it in Common is the obvious place to put it semantically, but since BizInvoke can't be used in it, I just opted for function pointers (which isn't too bad in this case).

Having it as a source generator in the future would eliminate any dependency nonsense (the most any kind of dependencies are needed would be some attribute and CallingConventionAdapter which could just be defined in Common which everything would be happy with), with of course any performance benefits that come with doing that (I could imagine being in the same assembly could let the JIT better optimize the code in any case).

BizInvoke uses calli

To be clear, I meant it would be faster than DllImport. It should perform around the same as BizInvoke, maybe slightly faster than BizInvoke (I assume same assembly offers better optimization / inlining) and slightly less instantiation time.

This comment has been minimized.

Sign in to view

Copy link
@nattthebear

nattthebear May 19, 2024

Contributor

LibraryImport is not suitable as a replacement for BizInvoke in general due to the IMonitor and other requirements which disallow static instantiation, which is mainly relevant for waterboxing where it is primarily used.

Yeah, I was thinking only about these static cases.

Different filename on Linux.

Many applications handle differences between libfoo.so and foo.dll without writing their own entire import layer, there must be a way.

This comment has been minimized.

Sign in to view

Copy link
@YoshiRulz

YoshiRulz May 19, 2024

Member

Mono checks for a few variants like libfoo.dll.so (you can set a debug env. var. or simply strace to see this), but they don't work with the version suffix. I've revisited it a few times over the years and passing the correct filename to DynamicLibraryImportResolver is still the simplest way.
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

namespace BizHawk.Common
{
public static unsafe class LibZstd
{
static LibZstd()
{
var resolver = new DynamicLibraryImportResolver(
OSTailoredCode.IsUnixHost ? "libzstd.so.1" : "libzstd.dll", hasLimitedLifetime: false);
ZSTD_isError = (delegate* unmanaged[Cdecl]<nuint, uint>)resolver.GetProcAddrOrThrow(nameof(ZSTD_isError));
ZSTD_getErrorName = (delegate* unmanaged[Cdecl]<nuint, IntPtr>)resolver.GetProcAddrOrThrow(nameof(ZSTD_getErrorName));
ZSTD_minCLevel = (delegate* unmanaged[Cdecl]<int>)resolver.GetProcAddrOrThrow(nameof(ZSTD_minCLevel));
ZSTD_maxCLevel = (delegate* unmanaged[Cdecl]<int>)resolver.GetProcAddrOrThrow(nameof(ZSTD_maxCLevel));
ZSTD_createCStream = (delegate* unmanaged[Cdecl]<IntPtr>)resolver.GetProcAddrOrThrow(nameof(ZSTD_createCStream));
ZSTD_freeCStream = (delegate* unmanaged[Cdecl]<IntPtr, nuint>)resolver.GetProcAddrOrThrow(nameof(ZSTD_freeCStream));
ZSTD_initCStream = (delegate* unmanaged[Cdecl]<IntPtr, int, nuint>)resolver.GetProcAddrOrThrow(nameof(ZSTD_initCStream));
ZSTD_compressStream = (delegate* unmanaged[Cdecl]<IntPtr, StreamBuffer*, StreamBuffer*, nuint>)resolver.GetProcAddrOrThrow(nameof(ZSTD_compressStream));
ZSTD_flushStream = (delegate* unmanaged[Cdecl]<IntPtr, StreamBuffer*, nuint>)resolver.GetProcAddrOrThrow(nameof(ZSTD_flushStream));
ZSTD_endStream = (delegate* unmanaged[Cdecl]<IntPtr, StreamBuffer*, nuint>)resolver.GetProcAddrOrThrow(nameof(ZSTD_endStream));
ZSTD_createDStream = (delegate* unmanaged[Cdecl]<IntPtr>)resolver.GetProcAddrOrThrow(nameof(ZSTD_createDStream));
ZSTD_freeDStream = (delegate* unmanaged[Cdecl]<IntPtr, nuint>)resolver.GetProcAddrOrThrow(nameof(ZSTD_freeDStream));
ZSTD_initDStream = (delegate* unmanaged[Cdecl]<IntPtr, nuint>)resolver.GetProcAddrOrThrow(nameof(ZSTD_initDStream));
ZSTD_decompressStream = (delegate* unmanaged[Cdecl]<IntPtr, StreamBuffer*, StreamBuffer*, nuint>)resolver.GetProcAddrOrThrow(nameof(ZSTD_decompressStream));
}

private static readonly delegate* unmanaged[Cdecl]<nuint, uint> ZSTD_isError;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static uint IsError(nuint code) => ZSTD_isError(code);

private static readonly delegate* unmanaged[Cdecl]<nuint, IntPtr> ZSTD_getErrorName;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static IntPtr GetErrorName(nuint code) => ZSTD_getErrorName(code);

private static readonly delegate* unmanaged[Cdecl]<int> ZSTD_minCLevel;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static int MinCLevel() => ZSTD_minCLevel();

private static readonly delegate* unmanaged[Cdecl]<int> ZSTD_maxCLevel;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static int MaxCLevel() => ZSTD_maxCLevel();

[StructLayout(LayoutKind.Sequential)]
public struct StreamBuffer
{
public IntPtr Ptr;
public nuint Size;
public nuint Pos;
}

private static readonly delegate* unmanaged[Cdecl]<IntPtr> ZSTD_createCStream;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static IntPtr CreateCStream() => ZSTD_createCStream();

private static readonly delegate* unmanaged[Cdecl]<IntPtr, nuint> ZSTD_freeCStream;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static nuint FreeCStream(IntPtr zcs) => ZSTD_freeCStream(zcs);

private static readonly delegate* unmanaged[Cdecl]<IntPtr, int, nuint> ZSTD_initCStream;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static nuint InitCStream(IntPtr zcs, int compressionLevel) => ZSTD_initCStream(zcs, compressionLevel);

private static readonly delegate* unmanaged[Cdecl]<IntPtr, StreamBuffer*, StreamBuffer*, nuint> ZSTD_compressStream;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static nuint CompressStream(IntPtr zcs, ref StreamBuffer output, ref StreamBuffer input)
{
fixed (StreamBuffer* outputPtr = &output, inputPtr = &input)
{
return ZSTD_compressStream(zcs, outputPtr, inputPtr);
}
}

private static readonly delegate* unmanaged[Cdecl]<IntPtr, StreamBuffer*, nuint> ZSTD_flushStream;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static nuint FlushStream(IntPtr zcs, ref StreamBuffer output)
{
fixed (StreamBuffer* outputPtr = &output)
{
return ZSTD_flushStream(zcs, outputPtr);
}
}

private static readonly delegate* unmanaged[Cdecl]<IntPtr, StreamBuffer*, nuint> ZSTD_endStream;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static nuint EndStream(IntPtr zcs, ref StreamBuffer output)
{
fixed (StreamBuffer* outputPtr = &output)
{
return ZSTD_endStream(zcs, outputPtr);
}
}

private static readonly delegate* unmanaged[Cdecl]<IntPtr> ZSTD_createDStream;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static IntPtr CreateDStream() => ZSTD_createDStream();

private static readonly delegate* unmanaged[Cdecl]<IntPtr, nuint> ZSTD_freeDStream;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static nuint FreeDStream(IntPtr zds) => ZSTD_freeDStream(zds);

private static readonly delegate* unmanaged[Cdecl]<IntPtr, nuint> ZSTD_initDStream;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static nuint InitDStream(IntPtr zds) => ZSTD_initDStream(zds);

private static readonly delegate* unmanaged[Cdecl]<IntPtr, StreamBuffer*, StreamBuffer*, nuint> ZSTD_decompressStream;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static nuint DecompressStream(IntPtr zds, ref StreamBuffer output, ref StreamBuffer input)
{
fixed (StreamBuffer* outputPtr = &output, inputPtr = &input)
{
return ZSTD_decompressStream(zds, outputPtr, inputPtr);
}
}
}
}
Loading

0 comments on commit c2f549d

Please sign in to comment.