From a20c3c3ccb7ba3eeebb8294a282b8d9cd8f029ab Mon Sep 17 00:00:00 2001 From: nattthebear Date: Mon, 25 May 2020 19:46:34 -0400 Subject: [PATCH] Change waterbox to use memory reserve semantics If a particular core instance doesn't use up too much memory, then we don't charge the host OS for that memory. The strange thing here is that, testing locally on Windows 10, we were already getting the desired semantics even with SEC_COMMIT and not SEC_RESERVE. This commit is still important because: 1) Windows might be providing me an optimization it can't guarantee, 2) Linux might not be able to provide the same optimization 3) In any event, this patch also trims down xor snapshots to match the actual needed size. --- src/BizHawk.BizInvoke/IMemoryBlockPal.cs | 17 ++- src/BizHawk.BizInvoke/MemoryBlock.cs | 123 ++++++++++----- src/BizHawk.BizInvoke/MemoryBlockUnixPal.cs | 45 ++++-- .../MemoryBlockWindowsPal.cs | 25 ++- src/BizHawk.BizInvoke/POSIXLibC.cs | 4 +- src/BizHawk.BizInvoke/WaterboxUtils.cs | 9 +- .../MemoryDomainStream.cs | 12 +- .../Waterbox/NymaCore.Settings.cs | 1 + .../Waterbox/WaterboxUtils.cs | 142 ------------------ 9 files changed, 179 insertions(+), 199 deletions(-) delete mode 100644 src/BizHawk.Emulation.Cores/Waterbox/WaterboxUtils.cs diff --git a/src/BizHawk.BizInvoke/IMemoryBlockPal.cs b/src/BizHawk.BizInvoke/IMemoryBlockPal.cs index d9a3354042..e43d7b07f8 100644 --- a/src/BizHawk.BizInvoke/IMemoryBlockPal.cs +++ b/src/BizHawk.BizInvoke/IMemoryBlockPal.cs @@ -8,16 +8,27 @@ namespace BizHawk.BizInvoke public interface IMemoryBlockPal : IDisposable { /// - /// Map in the memory area at the predetermined address + /// Map in the memory area at the predetermined address. uncommitted space should be unreadable. + /// For all other space, there is no requirement on initial protection value; + /// correct protections will be applied via Protect() immediately after this call. /// void Activate(); /// - /// Unmap the memory area + /// Unmap the memory area from memory. All data needs to be preserved for next load. /// void Deactivate(); /// - /// Change protection on some addresses, guaranteed to be page aligned and in the memory area + /// Change protection on [start, start + size), guaranteed to be page aligned and in the committed area. + /// Will only be called when active. /// void Protect(ulong start, ulong size, MemoryBlock.Protection prot); + /// + /// mark [Block.Start, Block.Start + length) as committed. Always greater than a previous length; + /// no uncommitting is allowed. + /// Will only be called when active. + /// there is no requirement on initial protection value of any committed memory (newly or otherwise) + /// after this call; protections will be applied via Protect() immediately after this call. + /// + void Commit(ulong length); } } diff --git a/src/BizHawk.BizInvoke/MemoryBlock.cs b/src/BizHawk.BizInvoke/MemoryBlock.cs index fe55abe761..40533144ff 100644 --- a/src/BizHawk.BizInvoke/MemoryBlock.cs +++ b/src/BizHawk.BizInvoke/MemoryBlock.cs @@ -27,10 +27,24 @@ namespace BizHawk.BizInvoke private IMemoryBlockPal _pal; - /// stores last set memory protection value for each page - protected readonly Protection[] _pageData; + /// + /// Size that has been committed to actual underlying RAM. Never shrinks. Private because + /// it should be transparent to the caller. ALWAYS ALIGNED. + /// + private ulong CommittedSize; - /// end address of the memory block (not part of the block; class invariant: equal to + ) + /// + /// The last CommittedSize that was sent to the PAL layer when we were active. + /// TODO: Do we really need the ability to change protections while inactive? + /// + private ulong LastActiveCommittedSize; + + /// stores last set memory protection value for each page + private readonly Protection[] _pageData; + + /// + /// end address of the memory block (not part of the block; class invariant: equal to + ) + /// public readonly ulong EndExclusive; /// total size of the memory block @@ -40,23 +54,23 @@ namespace BizHawk.BizInvoke public readonly ulong Start; /// snapshot for XOR buffer - protected byte[] _snapshot; + private byte[] _snapshot; /// true if this is currently swapped in - public bool Active { get; protected set; } + public bool Active { get; private set; } - public byte[] XorHash { get; protected set; } + public byte[] XorHash { get; private set; } /// get a page index within the block - protected int GetPage(ulong addr) + private int GetPage(ulong addr) { - if (addr < Start || EndExclusive <= addr) + if (addr < Start || addr >= EndExclusive) throw new ArgumentOutOfRangeException(nameof(addr), addr, "invalid address"); return (int) ((addr - Start) >> WaterboxUtils.PageShift); } /// get a start address for a page index within the block - protected ulong GetStartAddr(int page) => ((ulong) page << WaterboxUtils.PageShift) + Start; + private ulong GetStartAddr(int page) => ((ulong) page << WaterboxUtils.PageShift) + Start; /// /// Get a stream that can be used to read or write from part of the block. Does not check for or change ! @@ -101,6 +115,11 @@ namespace BizHawk.BizInvoke if (Active) throw new InvalidOperationException("Already active"); _pal.Activate(); + if (CommittedSize > LastActiveCommittedSize) + { + _pal.Commit(CommittedSize); + LastActiveCommittedSize = CommittedSize; + } ProtectAll(); Active = true; } @@ -117,7 +136,10 @@ namespace BizHawk.BizInvoke Active = false; } - /// take a hash of the current full contents of the block, including unreadable areas + /// + /// take a hash of the current full contents of the block, including unreadable areas + /// but not uncommitted areas + /// /// /// is or failed to make memory read-only /// @@ -125,9 +147,10 @@ namespace BizHawk.BizInvoke { if (!Active) throw new InvalidOperationException("Not active"); - // temporarily switch the entire block to `R` - _pal.Protect(Start, Size, Protection.R); - var ret = WaterboxUtils.Hash(GetStream(Start, Size, false)); + // temporarily switch the committed parts to `R` so we can read them + if (CommittedSize > 0) + _pal.Protect(Start, CommittedSize, Protection.R); + var ret = WaterboxUtils.Hash(GetStream(Start, CommittedSize, false)); ProtectAll(); return ret; } @@ -139,29 +162,51 @@ namespace BizHawk.BizInvoke { if (length == 0) return; + + // Note: asking for prot.none on memory that was not previously committed, commits it + + var computedStart = WaterboxUtils.AlignDown(start); + var computedEnd = WaterboxUtils.AlignUp(start + length); + var computedLength = computedEnd - computedStart; + int pstart = GetPage(start); int pend = GetPage(start + length - 1); for (int i = pstart; i <= pend; i++) _pageData[i] = prot; // also store the value for later use + // potentially commit more memory + var minNewCommittedSize = computedEnd - Start; + if (minNewCommittedSize > CommittedSize) + { + CommittedSize = minNewCommittedSize; + } + if (Active) // it's legal to Protect() if we're not active; the information is just saved for the next activation { - var computedStart = WaterboxUtils.AlignDown(start); - var computedEnd = WaterboxUtils.AlignUp(start + length); - var computedLength = computedEnd - computedStart; - - _pal.Protect(computedStart, computedLength, prot); + if (CommittedSize > LastActiveCommittedSize) + { + _pal.Commit(CommittedSize); + LastActiveCommittedSize = CommittedSize; + ProtectAll(); + } + else + { + _pal.Protect(computedStart, computedLength, prot); + } } } /// restore all recorded protections private void ProtectAll() { + if (CommittedSize == 0) + return; int ps = 0; - for (int i = 0; i < _pageData.Length; i++) + int pageLimit = (int)(CommittedSize >> WaterboxUtils.PageShift); + for (int i = 0; i < pageLimit; i++) { - if (i == _pageData.Length - 1 || _pageData[i] != _pageData[i + 1]) + if (i == pageLimit - 1 || _pageData[i] != _pageData[i + 1]) { ulong zstart = GetStartAddr(ps); ulong zend = GetStartAddr(i + 1); @@ -177,18 +222,20 @@ namespace BizHawk.BizInvoke /// public void SaveXorSnapshot() { + // note: The snapshot only holds up to the current committed size. We compensate for that in xorstream if (_snapshot != null) throw new InvalidOperationException("Snapshot already taken"); if (!Active) throw new InvalidOperationException("Not active"); - // temporarily switch the entire block to `R`: in case some areas are unreadable, we don't want + // temporarily switch the entire committed area to `R`: in case some areas are unreadable, we don't want // that to complicate things - _pal.Protect(Start, Size, Protection.R); + if (CommittedSize > 0) + _pal.Protect(Start, CommittedSize, Protection.R); - _snapshot = new byte[Size]; + _snapshot = new byte[CommittedSize]; var ds = new MemoryStream(_snapshot, true); - var ss = GetStream(Start, Size, false); + var ss = GetStream(Start, CommittedSize, false); ss.CopyTo(ds); XorHash = WaterboxUtils.Hash(_snapshot); @@ -242,7 +289,8 @@ namespace BizHawk.BizInvoke get => _pos; set { - if (value < 0 || _length < value) throw new ArgumentOutOfRangeException(); + if (value < 0 || value > _length) + throw new ArgumentOutOfRangeException(); _pos = value; } } @@ -259,11 +307,11 @@ namespace BizHawk.BizInvoke { if (!_readable) throw new InvalidOperationException(); - if (count < 0 || buffer.Length < count + offset) + if (count < 0 || offset + count > buffer.Length) throw new ArgumentOutOfRangeException(); EnsureNotDisposed(); - count = (int) Math.Min(count, _length - _pos); + count = (int)Math.Min(count, _length - _pos); Marshal.Copy(Z.SS(_ptr + _pos), buffer, offset, count); _pos += count; return count; @@ -298,7 +346,7 @@ namespace BizHawk.BizInvoke { if (!_writable) throw new InvalidOperationException(); - if (count < 0 || _length - _pos < count || buffer.Length < count + offset) + if (count < 0 || _pos + count > _length || offset + count > buffer.Length) throw new ArgumentOutOfRangeException(); EnsureNotDisposed(); @@ -313,7 +361,7 @@ namespace BizHawk.BizInvoke : base(readable, writable, ptr, length, owner) { _initial = initial; - _offset = (int) offset; + _offset = (int)offset; } /// the initial data to XOR against for both reading and writing @@ -324,7 +372,7 @@ namespace BizHawk.BizInvoke public override int Read(byte[] buffer, int offset, int count) { - var pos = (int) Position; + var pos = (int)Position; count = base.Read(buffer, offset, count); XorTransform(_initial, _offset + pos, buffer, offset, count); return count; @@ -332,24 +380,29 @@ namespace BizHawk.BizInvoke public override void Write(byte[] buffer, int offset, int count) { - var pos = (int) Position; - if (count < 0 || Length - pos < count || buffer.Length < count + offset) throw new ArgumentOutOfRangeException(); + var pos = (int)Position; + if (count < 0 || pos + count > Length || offset + count > buffer.Length) + throw new ArgumentOutOfRangeException(); // is mutating the buffer passed to Stream.Write kosher? XorTransform(_initial, _offset + pos, buffer, offset, count); base.Write(buffer, offset, count); } - /// bounds check already done by calling method i.e. in base.Read (for ) or in private static unsafe void XorTransform(byte[] source, int sourceOffset, byte[] dest, int destOffset, int length) { + // bounds checks on dest have been done already, but not on source + // If a xorsnapshot was created and then the heap was later expanded, those other bytes are effectively 0 + // TODO: C compilers can make this pretty snappy, but can the C# jitter? Or do we need intrinsics fixed (byte* _s = source, _d = dest) { byte* s = _s + sourceOffset; byte* d = _d + destOffset; - byte* sEnd = s + length; - while (s < sEnd) *d++ ^= *s++; + byte* sEnd = _s + Math.Min(sourceOffset + length, source.Length); + while (s < sEnd) + *d++ ^= *s++; + // for anything left in dest past source.Length, we need not transform at all } } } diff --git a/src/BizHawk.BizInvoke/MemoryBlockUnixPal.cs b/src/BizHawk.BizInvoke/MemoryBlockUnixPal.cs index 961b414ac8..e48a64d521 100644 --- a/src/BizHawk.BizInvoke/MemoryBlockUnixPal.cs +++ b/src/BizHawk.BizInvoke/MemoryBlockUnixPal.cs @@ -10,24 +10,23 @@ namespace BizHawk.BizInvoke private int _fd = -1; private ulong _start; private ulong _size; + private ulong _committedSize; /// /// Reserve bytes to later be swapped in, but do not map them /// /// eventual mapped address /// - /// failed to get file descriptor (never thrown as is thrown first) - /// always + /// + /// failed to get file descriptor + /// public MemoryBlockUnixPal(ulong start, ulong size) { _start = start; _size = size; - throw new NotImplementedException($"{nameof(MemoryBlockUnixPal)} ctor"); - #if false _fd = memfd_create("MemoryBlockUnix", 0); if (_fd == -1) throw new InvalidOperationException($"{nameof(memfd_create)}() returned -1"); - #endif } public void Dispose() @@ -46,27 +45,45 @@ namespace BizHawk.BizInvoke public void Activate() { - var ptr = mmap(Z.US(_start), Z.UU(_size), MemoryProtection.Read | MemoryProtection.Write | MemoryProtection.Execute, 16, _fd, IntPtr.Zero); - if (ptr != Z.US(_start)) - throw new InvalidOperationException($"{nameof(mmap)}() returned NULL or the wrong pointer"); + if (_committedSize > 0) + { + var ptr = mmap(Z.US(_start), Z.UU(_committedSize), + MemoryProtection.Read | MemoryProtection.Write | MemoryProtection.Execute, + 16, // MAP_FIXED + _fd, IntPtr.Zero); + if (ptr != Z.US(_start)) + throw new InvalidOperationException($"{nameof(mmap)}() returned NULL or the wrong pointer"); + } } public void Deactivate() { - var exitCode = munmap(Z.US(_start), Z.UU(_size)); - if (exitCode != 0) - throw new InvalidOperationException($"{nameof(munmap)}() returned {exitCode}"); + if (_committedSize > 0) + { + var errorCode = munmap(Z.US(_start), Z.UU(_committedSize)); + if (errorCode != 0) + throw new InvalidOperationException($"{nameof(munmap)}() returned {errorCode}"); + } + } + + public void Commit(ulong length) + { + Deactivate(); + var errorCode = ftruncate(_fd, Z.US(length)); + if (errorCode != 0) + throw new InvalidOperationException($"{nameof(ftruncate)}() returned {errorCode}"); + Activate(); } public void Protect(ulong start, ulong size, Protection prot) { - var exitCode = mprotect( + var errorCode = mprotect( Z.US(start), Z.UU(size), prot.ToMemoryProtection() ); - if (exitCode != 0) - throw new InvalidOperationException($"{nameof(mprotect)}() returned {exitCode}!"); + if (errorCode != 0) + throw new InvalidOperationException($"{nameof(mprotect)}() returned {errorCode}!"); } } } diff --git a/src/BizHawk.BizInvoke/MemoryBlockWindowsPal.cs b/src/BizHawk.BizInvoke/MemoryBlockWindowsPal.cs index b08d9edd86..33cdd73ba2 100644 --- a/src/BizHawk.BizInvoke/MemoryBlockWindowsPal.cs +++ b/src/BizHawk.BizInvoke/MemoryBlockWindowsPal.cs @@ -25,7 +25,7 @@ namespace BizHawk.BizInvoke _handle = Kernel32.CreateFileMapping( Kernel32.INVALID_HANDLE_VALUE, IntPtr.Zero, - Kernel32.FileMapProtection.PageExecuteReadWrite | Kernel32.FileMapProtection.SectionCommit, + Kernel32.FileMapProtection.PageExecuteReadWrite | Kernel32.FileMapProtection.SectionReserve, (uint)(_size >> 32), (uint)_size, null @@ -63,6 +63,12 @@ namespace BizHawk.BizInvoke throw new InvalidOperationException($"{nameof(Kernel32.VirtualProtect)}() returned FALSE!"); } + public void Commit(ulong length) + { + if (Kernel32.VirtualAlloc(Z.UU(_start), Z.UU(length), Kernel32.AllocationType.MEM_COMMIT, Kernel32.MemoryProtection.READWRITE) != Z.UU(_start)) + throw new InvalidOperationException($"{nameof(Kernel32.VirtualAlloc)}() returned NULL!"); + } + private static Kernel32.MemoryProtection GetKernelMemoryProtectionValue(Protection prot) { Kernel32.MemoryProtection p; @@ -94,10 +100,27 @@ namespace BizHawk.BizInvoke private static class Kernel32 { + [DllImport("kernel32.dll", SetLastError = true)] + public static extern UIntPtr VirtualAlloc(UIntPtr lpAddress, UIntPtr dwSize, + AllocationType flAllocationType, MemoryProtection flProtect); + [DllImport("kernel32.dll", SetLastError = true)] public static extern bool VirtualProtect(UIntPtr lpAddress, UIntPtr dwSize, MemoryProtection flNewProtect, out MemoryProtection lpflOldProtect); + [Flags] + public enum AllocationType : uint + { + MEM_COMMIT = 0x00001000, + MEM_RESERVE = 0x00002000, + MEM_RESET = 0x00080000, + MEM_RESET_UNDO = 0x1000000, + MEM_LARGE_PAGES = 0x20000000, + MEM_PHYSICAL = 0x00400000, + MEM_TOP_DOWN = 0x00100000, + MEM_WRITE_WATCH = 0x00200000 + } + [Flags] public enum MemoryProtection : uint { diff --git a/src/BizHawk.BizInvoke/POSIXLibC.cs b/src/BizHawk.BizInvoke/POSIXLibC.cs index db3e7a956a..67ea245dfd 100644 --- a/src/BizHawk.BizInvoke/POSIXLibC.cs +++ b/src/BizHawk.BizInvoke/POSIXLibC.cs @@ -25,10 +25,12 @@ namespace BizHawk.BizInvoke [DllImport("libc.so.6")] public static extern int munmap(IntPtr addr, UIntPtr length); + [DllImport("libc.so.6")] + public static extern int ftruncate(int fd, IntPtr length); /// 32-bit signed int [Flags] - public enum MemoryProtection { None = 0x0, Read = 0x1, Write = 0x2, Execute = 0x4 } + public enum MemoryProtection : int { None = 0x0, Read = 0x1, Write = 0x2, Execute = 0x4 } public static MemoryProtection ToMemoryProtection(this Protection prot) { diff --git a/src/BizHawk.BizInvoke/WaterboxUtils.cs b/src/BizHawk.BizInvoke/WaterboxUtils.cs index 18b727ecc8..3c69c447af 100644 --- a/src/BizHawk.BizInvoke/WaterboxUtils.cs +++ b/src/BizHawk.BizInvoke/WaterboxUtils.cs @@ -11,10 +11,10 @@ namespace BizHawk.BizInvoke /// public static void CopySome(Stream src, Stream dst, long len) { - var buff = new byte[4096]; + var buff = new byte[65536]; while (len > 0) { - int r = src.Read(buff, 0, (int)Math.Min(len, 4096)); + int r = src.Read(buff, 0, (int)Math.Min(len, 65536)); dst.Write(buff, 0, r); len -= r; } @@ -42,6 +42,11 @@ namespace BizHawk.BizInvoke } } + public static long Timestamp() + { + return DateTime.UtcNow.Ticks; + } + /// /// system page size /// diff --git a/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomainStream.cs b/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomainStream.cs index 123fc600d2..056ae122fd 100644 --- a/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomainStream.cs +++ b/src/BizHawk.Emulation.Common/Base Implementations/MemoryDomainStream.cs @@ -11,6 +11,7 @@ namespace BizHawk.Emulation.Common _d = d; } private readonly MemoryDomain _d; + private long _position; public override bool CanRead => true; @@ -20,7 +21,16 @@ namespace BizHawk.Emulation.Common public override long Length => _d.Size; - public override long Position { get; set; } + public override long Position + { + get => _position; + set + { + if (value < 0 || value > _d.Size) + throw new IOException("Position out of range"); + _position = value; + } + } public override void Flush() { diff --git a/src/BizHawk.Emulation.Cores/Waterbox/NymaCore.Settings.cs b/src/BizHawk.Emulation.Cores/Waterbox/NymaCore.Settings.cs index 3e61ddbd4c..2bc5e01b84 100644 --- a/src/BizHawk.Emulation.Cores/Waterbox/NymaCore.Settings.cs +++ b/src/BizHawk.Emulation.Cores/Waterbox/NymaCore.Settings.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; using System.Runtime.InteropServices; using System.Text; +using BizHawk.BizInvoke; using BizHawk.Common; using BizHawk.Emulation.Common; diff --git a/src/BizHawk.Emulation.Cores/Waterbox/WaterboxUtils.cs b/src/BizHawk.Emulation.Cores/Waterbox/WaterboxUtils.cs deleted file mode 100644 index 4e9dbc969b..0000000000 --- a/src/BizHawk.Emulation.Cores/Waterbox/WaterboxUtils.cs +++ /dev/null @@ -1,142 +0,0 @@ -using System; -using System.IO; -using System.Security.Cryptography; - -namespace BizHawk.Emulation.Cores.Waterbox -{ - public static class WaterboxUtils - { - /// - /// copy `len` bytes from `src` to `dest` - /// - public static void CopySome(Stream src, Stream dst, long len) - { - var buff = new byte[65536]; - while (len > 0) - { - int r = src.Read(buff, 0, (int)Math.Min(len, 65536)); - dst.Write(buff, 0, r); - len -= r; - } - } - - public static byte[] Hash(byte[] data) - { - using var h = SHA1.Create(); - return h.ComputeHash(data); - } - - public static byte[] Hash(Stream s) - { - using var h = SHA1.Create(); - return h.ComputeHash(s); - } - - public static unsafe void ZeroMemory(IntPtr mem, long length) - { - byte* p = (byte*)mem; - byte* end = p + length; - while (p < end) - { - *p++ = 0; - } - } - - public static long Timestamp() - { - return DateTime.UtcNow.Ticks; - } - - /// - /// system page size - /// - public static int PageSize { get; } - - /// - /// bitshift corresponding to PageSize - /// - public static int PageShift { get; } - /// - /// bitmask corresponding to PageSize - /// - public static ulong PageMask { get; } - - static WaterboxUtils() - { - int p = PageSize = Environment.SystemPageSize; - while (p != 1) - { - p >>= 1; - PageShift++; - } - PageMask = (ulong)(PageSize - 1); - } - - /// - /// true if addr is aligned - /// - public static bool Aligned(ulong addr) - { - return (addr & PageMask) == 0; - } - - /// - /// align address down to previous page boundary - /// - public static ulong AlignDown(ulong addr) - { - return addr & ~PageMask; - } - - /// - /// align address up to next page boundary - /// - public static ulong AlignUp(ulong addr) - { - return ((addr - 1) | PageMask) + 1; - } - - /// - /// return the minimum number of pages needed to hold size - /// - public static int PagesNeeded(ulong size) - { - return (int)((size + PageMask) >> PageShift); - } - } - - // C# is annoying: arithmetic operators for native ints are not exposed. - // So we store them as long/ulong instead in many places, and use these helpers - // to convert to IntPtr when needed - - public static class Z - { - public static IntPtr US(ulong l) - { - if (IntPtr.Size == 8) - return (IntPtr)(long)l; - return (IntPtr)(int)l; - } - - public static UIntPtr UU(ulong l) - { - if (UIntPtr.Size == 8) - return (UIntPtr)l; - return (UIntPtr)(uint)l; - } - - public static IntPtr SS(long l) - { - if (IntPtr.Size == 8) - return (IntPtr)l; - return (IntPtr)(int)l; - } - - public static UIntPtr SU(long l) - { - if (UIntPtr.Size == 8) - return (UIntPtr)(ulong)l; - return (UIntPtr)(uint)l; - } - } -}