From c2143584bc1dcdd9cf1133a35d5477a49a07659d Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Thu, 24 Dec 2020 14:21:17 -0800 Subject: [PATCH] fix(core): Fixes BufferWriter Seek and removes World.SaveBuffers (#359) - [X] Fixes an issue in BufferWriter where if SeekOrigin.End is used, it will yield the wrong index. - [X] Changes BinaryFileWriter to dispose pattern - [X] Removes World.SaveBuffers. They were dangerous and should be done in-line while the world is loading, even if it is slower - [X] Fixes BufferReader Seek too - [X] Changes BufferWriter to use uninitialized arrays to speed up resizing - [X] Changes World loading so it doesn't buffer the whole file, even if it makes things slower. It lowers allocations/fragmentation over all which is good. - [X] Changes World Loading to use uninitialized arrays and directly saves them to the SaveBuffer which eliminates having to open/load the files _again_ which is dangerous. Also looping through items while the world is running is dangerous since async references can and will cause exceptions. Bumps release version --- .../Server/Serialization/BinaryFileWriter.cs | 33 ++-- Projects/Server/Serialization/BufferReader.cs | 63 +++++--- Projects/Server/Serialization/BufferWriter.cs | 71 +++++++-- Projects/Server/World/World.cs | 145 +++++++----------- 4 files changed, 178 insertions(+), 134 deletions(-) diff --git a/Projects/Server/Serialization/BinaryFileWriter.cs b/Projects/Server/Serialization/BinaryFileWriter.cs index 193673fcd..639c12043 100644 --- a/Projects/Server/Serialization/BinaryFileWriter.cs +++ b/Projects/Server/Serialization/BinaryFileWriter.cs @@ -13,36 +13,38 @@ * along with this program. If not, see . * *************************************************************************/ +using System; using System.IO; namespace Server { - public class BinaryFileWriter : BufferWriter + public class BinaryFileWriter : BufferWriter, IDisposable { - private readonly Stream m_File; - private long m_Position; + private readonly Stream _file; + private long _position; - public BinaryFileWriter(string filename, bool prefixStr) : base(prefixStr) => - m_File = new FileStream(filename, FileMode.Create, FileAccess.Write, FileShare.None); + public BinaryFileWriter(string filename, bool prefixStr) : + this(new FileStream(filename, FileMode.Create, FileAccess.Write, FileShare.None), prefixStr) + {} public BinaryFileWriter(Stream stream, bool prefixStr) : base(prefixStr) { - m_File = stream; - m_Position = m_File.Position; + _file = stream; + _position = _file.Position; } - public override long Position => m_Position + Index; + public override long Position => _position + Index; - protected override int BufferSize => 512; + protected override int BufferSize => 81920; public override void Flush() { if (Index > 0) { - m_Position += Index; + _position += Index; - m_File.Write(Buffer, 0, (int)Index); + _file.Write(Buffer, 0, (int)Index); Index = 0; } } @@ -54,14 +56,19 @@ namespace Server Flush(); } - m_File.Close(); + _file.Close(); } public override long Seek(long offset, SeekOrigin origin) { Flush(); - return m_Position = m_File.Seek(offset, origin); + return _position = _file.Seek(offset, origin); + } + + public void Dispose() + { + Close(); } } } diff --git a/Projects/Server/Serialization/BufferReader.cs b/Projects/Server/Serialization/BufferReader.cs index 250f0ee82..8860c851c 100644 --- a/Projects/Server/Serialization/BufferReader.cs +++ b/Projects/Server/Serialization/BufferReader.cs @@ -16,6 +16,7 @@ using System; using System.Buffers.Binary; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Net; using System.Text; @@ -27,7 +28,11 @@ namespace Server { private readonly Encoding _encoding; private byte[] _buffer; - public int Position { get; private set; } + private int _position; + + public int Position => _position; + + public byte[] Buffer => _buffer; public BufferReader(byte[] buffer) { @@ -35,6 +40,13 @@ namespace Server _encoding = Utility.UTF8; } + public void SwapBuffers(byte[] newBuffer, out byte[] oldBuffer) + { + oldBuffer = _buffer; + _buffer = newBuffer; + _position = 0; + } + public string ReadString() { if (!ReadBool()) @@ -49,7 +61,7 @@ namespace Server } var s = _encoding.GetString(_buffer.AsSpan(Position, length)); - Position += length; + _position += length; return s; } @@ -64,64 +76,64 @@ namespace Server public long ReadLong() { var v = BinaryPrimitives.ReadInt64LittleEndian(_buffer.AsSpan(Position, 8)); - Position += 8; + _position += 8; return v; } public ulong ReadULong() { var v = BinaryPrimitives.ReadUInt64LittleEndian(_buffer.AsSpan(Position, 8)); - Position += 8; + _position += 8; return v; } public int ReadInt() { var v = BinaryPrimitives.ReadInt32LittleEndian(_buffer.AsSpan(Position, 4)); - Position += 4; + _position += 4; return v; } public uint ReadUInt() { var v = BinaryPrimitives.ReadUInt32LittleEndian(_buffer.AsSpan(Position, 4)); - Position += 4; + _position += 4; return v; } public short ReadShort() { var v = BinaryPrimitives.ReadInt16LittleEndian(_buffer.AsSpan(Position, 2)); - Position += 2; + _position += 2; return v; } public ushort ReadUShort() { var v = BinaryPrimitives.ReadUInt16LittleEndian(_buffer.AsSpan(Position, 2)); - Position += 2; + _position += 2; return v; } public double ReadDouble() { var v = BinaryPrimitives.ReadDoubleLittleEndian(_buffer.AsSpan(Position, 8)); - Position += 8; + _position += 8; return v; } public float ReadFloat() { var v = BinaryPrimitives.ReadSingleLittleEndian(_buffer.AsSpan(Position, 4)); - Position += 4; + _position += 4; return v; } - public byte ReadByte() => _buffer[Position++]; + public byte ReadByte() => _buffer[_position++]; - public sbyte ReadSByte() => (sbyte)_buffer[Position++]; + public sbyte ReadSByte() => (sbyte)_buffer[_position++]; - public bool ReadBool() => _buffer[Position++] != 0; + public bool ReadBool() => _buffer[_position++] != 0; public int ReadEncodedInt() { @@ -217,18 +229,31 @@ namespace Server } _buffer.AsSpan(Position, length).CopyTo(buffer); - Position += length; + _position += length; return length; } public virtual int Seek(int offset, SeekOrigin origin) { - return origin switch + Debug.Assert( + origin != SeekOrigin.End || offset <= 0 && offset > -_buffer.Length, + "Attempting to seek to an invalid position using SeekOrigin.End" + ); + Debug.Assert( + origin != SeekOrigin.Begin || offset >= 0 && offset < _buffer.Length, + "Attempting to seek to an invalid position using SeekOrigin.Begin" + ); + Debug.Assert( + origin != SeekOrigin.Current || _position + offset >= 0 && _position + offset < _buffer.Length, + "Attempting to seek to an invalid position using SeekOrigin.Current" + ); + + return _position = Math.Max(0, origin switch { - SeekOrigin.Current => Position += offset, - SeekOrigin.End => Position = _buffer.Length - offset, - _ => Position = offset // Begin - }; + SeekOrigin.Current => _position + offset, + SeekOrigin.End => _buffer.Length + offset, + _ => offset // Begin + }); } } } diff --git a/Projects/Server/Serialization/BufferWriter.cs b/Projects/Server/Serialization/BufferWriter.cs index f4fae7a0b..25d64826d 100644 --- a/Projects/Server/Serialization/BufferWriter.cs +++ b/Projects/Server/Serialization/BufferWriter.cs @@ -15,6 +15,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Net; using System.Runtime.CompilerServices; @@ -26,8 +27,30 @@ namespace Server { private readonly Encoding m_Encoding; private readonly bool m_PrefixStrings; + private long _bytesWritten; + private long _index; + + protected long Index + { + get => _index; + set + { + if (value < 0 || value > _buffer.Length) + { + // If you are receiving this exception and your value is too large, you may need to use `Resize` + // If you are receiving this exception and your value is negative, you probably used Seek incorrectly. + throw new ArgumentOutOfRangeException(nameof(value)); + } + + _index = value; + + if (value > _bytesWritten) + { + _bytesWritten = value; + } + } + } - protected long Index { get; set; } private byte[] _buffer; public BufferWriter(byte[] buffer, bool prefixStr) @@ -37,18 +60,15 @@ namespace Server _buffer = buffer; } - public BufferWriter(bool prefixStr) + public BufferWriter(bool prefixStr) : this(0, prefixStr) { - m_PrefixStrings = prefixStr; - m_Encoding = Utility.UTF8; - _buffer = GC.AllocateUninitializedArray(BufferSize); } public BufferWriter(int count, bool prefixStr) { m_PrefixStrings = prefixStr; m_Encoding = Utility.UTF8; - _buffer = GC.AllocateUninitializedArray(count); + _buffer = GC.AllocateUninitializedArray(count < 1 ? BufferSize : count); } public virtual long Position => Index; @@ -70,7 +90,16 @@ namespace Server size = BufferSize; } - Array.Resize(ref _buffer, size); + if (size < _buffer.Length) + { + _bytesWritten = size; + } + + var newBuffer = GC.AllocateUninitializedArray(size); + _buffer.AsSpan(0, Math.Min(size, _buffer.Length)).CopyTo(newBuffer); + _buffer = newBuffer; + + // Array.Resize(ref _buffer, size); } public virtual void Flush() @@ -82,15 +111,12 @@ namespace Server } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private bool FlushIfNeeded(int amount) + private void FlushIfNeeded(int amount) { if (Index + amount > _buffer.Length) { Flush(); - return true; } - - return false; } public void Write(ReadOnlySpan bytes) @@ -113,12 +139,25 @@ namespace Server public virtual long Seek(long offset, SeekOrigin origin) { - return origin switch + Debug.Assert( + origin != SeekOrigin.End || offset <= 0 && offset > -_buffer.Length, + "Attempting to seek to an invalid position using SeekOrigin.End" + ); + Debug.Assert( + origin != SeekOrigin.Begin || offset >= 0 && offset < _buffer.Length, + "Attempting to seek to an invalid position using SeekOrigin.Begin" + ); + Debug.Assert( + origin != SeekOrigin.Current || Index + offset >= 0 && Index + offset < _buffer.Length, + "Attempting to seek to an invalid position using SeekOrigin.Current" + ); + + return Index = Math.Max(0, origin switch { - SeekOrigin.Current => Index += offset, - SeekOrigin.End => Index = _buffer.Length - offset, - _ => Index = offset // Begin - }; + SeekOrigin.Current => Index + offset, + SeekOrigin.End => _bytesWritten + offset, + _ => offset // Begin + }); } public void WriteEncodedInt(int value) diff --git a/Projects/Server/World/World.cs b/Projects/Server/World/World.cs index bcd16bd27..eda62e242 100644 --- a/Projects/Server/World/World.cs +++ b/Projects/Server/World/World.cs @@ -31,7 +31,6 @@ namespace Server { Initial, Loading, - WritingLoadBuffers, Running, Saving, WritingSave @@ -142,7 +141,7 @@ namespace Server { if (WorldState != WorldState.Saving) { - Console.WriteLine("Attempting to queue {0} for decay but the world is not saving", item); + WriteConsoleLine($"Attempting to queue {item} for decay but the world is not saving"); return; } @@ -191,22 +190,20 @@ namespace Server if (t?.IsAbstract != false) { - Console.WriteLine("failed"); + WriteConsoleLine("failed"); - Console.WriteLine( - "Error: Type '{0}' was {1}. Delete all of those types? (y/n)", - typeName, - t?.IsAbstract == true ? "marked abstract" : "not found" - ); + var issue = t?.IsAbstract == true ? "marked abstract" : "not found"; + + WriteConsoleLine($"Error: Type '{typeName}' was {issue}. Delete all of those types? (y/n)"); if (Console.ReadKey(true).Key == ConsoleKey.Y) { types.Add(null); - Console.Write("World: Loading..."); + WriteConsole("Loading..."); continue; } - Console.WriteLine("Types will not be deleted. An exception will be thrown."); + WriteConsoleLine("Types will not be deleted. An exception will be thrown."); throw new Exception($"Bad type '{typeName}'"); } @@ -290,35 +287,6 @@ namespace Server return map; } - private static void SaveBuffers(IIndexInfo indexInfo, List> entities) where T : class, ISerializable - { - var indexType = indexInfo.TypeName; - - string dataPath = Path.Combine("Saves", indexType, $"{indexType}.bin"); - - if (!File.Exists(dataPath)) - { - return; - } - - using FileStream bin = new FileStream(dataPath, FileMode.Open, FileAccess.Read, FileShare.Read); - BinaryReader reader = new BinaryReader(bin); - - foreach (var entry in entities) - { - T e = entry.Entity; - - if (e == null || e is IEntity entity && entity.Deleted) - { - continue; - } - - byte[] saveBuffer = GC.AllocateUninitializedArray(entry.Length); - reader.Read(saveBuffer, 0, entry.Length); - e.InitializeSaveBuffer(saveBuffer); - } - } - private static void LoadData(IIndexInfo indexInfo, List> entities) where T : class, ISerializable { var indexType = indexInfo.TypeName; @@ -331,11 +299,8 @@ namespace Server } using FileStream bin = new FileStream(dataPath, FileMode.Open, FileAccess.Read, FileShare.Read); - var buffer = bin.Length <= int.MaxValue ? GC.AllocateUninitializedArray((int)bin.Length) : new byte[bin.Length]; - bin.Read(buffer); - bin.Close(); - var br = new BufferReader(buffer); + BufferReader br = null; foreach (var entry in entities) { @@ -344,29 +309,42 @@ namespace Server // Skip this entry if (t == null) { - br.Seek(entry.Length, SeekOrigin.Current); + bin.Seek(entry.Length, SeekOrigin.Current); continue; } - t.Deserialize(br); - var end = entry.Position + entry.Length; - - if (br.Position != end) + var buffer = GC.AllocateUninitializedArray(entry.Length); + if (br == null) { - Console.WriteLine($"***** Bad deserialize on {t.GetType()} *****"); - Console.WriteLine( - $"Serialized object was {entry.Length} bytes, but {br.Position - entry.Position} bytes deserialized" + br = new BufferReader(buffer); + } + else + { + br.SwapBuffers(buffer, out _); + } + + bin.Read(buffer.AsSpan()); + + t.Deserialize(br); + + if (br.Position != entry.Length) + { + WriteConsoleLine($"***** Bad deserialize on {t.GetType()} *****"); + WriteConsoleLine( + $"Serialized object was {entry.Length} bytes, but {br.Position} bytes deserialized" ); - Console.WriteLine("Delete the object and continue? (y/n)"); + WriteConsoleLine("Delete the object and continue? (y/n)"); if (Console.ReadKey(true).Key != ConsoleKey.Y) { throw new Exception("Deserialization failed."); } t.Delete(); - - br.Seek((int)end, SeekOrigin.Begin); + } + else + { + t.InitializeSaveBuffer(buffer); } } } @@ -380,8 +358,7 @@ namespace Server WorldState = WorldState.Loading; - Console.Write("World: Loading..."); - + WriteConsole("Loading..."); var watch = Stopwatch.StartNew(); List> items; @@ -402,8 +379,6 @@ namespace Server EventSink.InvokeWorldLoad(); - WorldState = WorldState.WritingLoadBuffers; - ProcessSafetyQueues(); foreach (var item in Items.Values) @@ -433,15 +408,8 @@ namespace Server Mobiles.Count ); - // Async save buffers - ThreadPool.QueueUserWorkItem(state => - { - SaveBuffers(mobileIndexInfo, mobiles); - SaveBuffers(itemIndexInfo, items); - SaveBuffers(guildIndexInfo, guilds); - WorldState = WorldState.Running; - } - ); + + WorldState = WorldState.Running; } private static void ProcessSafetyQueues() @@ -467,7 +435,7 @@ namespace Server var message = $"Warning: Attempted to {action} {entity} during world save.{Environment.NewLine}This action could cause inconsistent state.{Environment.NewLine}It is strongly advised that the offending scripts be corrected."; - Console.WriteLine(message); + WriteConsoleLine(message); try { @@ -493,6 +461,7 @@ namespace Server public static void WriteFiles(object state) { var watch = Stopwatch.StartNew(); + WriteConsole("Writing snapshot..."); IIndexInfo itemIndexInfo = new EntityTypeIndex("Items"); IIndexInfo mobileIndexInfo = new EntityTypeIndex("Mobiles"); @@ -506,7 +475,7 @@ namespace Server m_DiskWriteHandle.Set(); - Console.WriteLine("World: Writing snapshot took {0:F1} seconds.", watch.Elapsed.TotalSeconds); + Console.WriteLine("done ({0:F2} seconds)", watch.Elapsed.TotalSeconds); Timer.DelayCall(FinishWorldSave); } @@ -523,9 +492,9 @@ namespace Server string tdbPath = Path.Combine(path, $"{typeName}.tdb"); string binPath = Path.Combine(path, $"{typeName}.bin"); - var idx = new BinaryFileWriter(idxPath, false); - var tdb = new BinaryFileWriter(tdbPath, false); - var bin = new BinaryFileWriter(binPath, true); + using var idx = new BinaryFileWriter(idxPath, false); + using var tdb = new BinaryFileWriter(tdbPath, false); + using var bin = new BinaryFileWriter(binPath, true); idx.Write(entities.Count); foreach (var e in entities.Values) @@ -546,10 +515,6 @@ namespace Server { tdb.Write(types[i].FullName); } - - idx.Close(); - tdb.Close(); - bin.Close(); } private static void SaveEntities(IEnumerable list, DateTime serializeStart) where T : class, ISerializable @@ -600,7 +565,7 @@ namespace Server var now = DateTime.UtcNow; - Console.Write("[{0}] World: Saving...", now.ToLongTimeString()); + WriteConsole("Saving..."); var watch = Stopwatch.StartNew(); @@ -619,20 +584,19 @@ namespace Server WorldState = WorldState.WritingSave; - ThreadPool.QueueUserWorkItem(WriteFiles); - watch.Stop(); var duration = watch.Elapsed.TotalSeconds; - - Console.WriteLine("Save done in {0:F2} seconds.", duration); + Console.WriteLine("done ({0:F2} seconds)", duration); // Only broadcast if it took at least 150ms if (duration >= 0.15) { - Broadcast(0x35, true, $"World save completed in {duration:F2} seconds."); + Broadcast(0x35, true, $"World Save completed in {duration:F2} seconds."); } + ThreadPool.QueueUserWorkItem(WriteFiles); + NetState.Resume(); } @@ -660,7 +624,6 @@ namespace Server goto case WorldState.Running; } - case WorldState.WritingLoadBuffers: case WorldState.Running: { if (serial.IsItem) @@ -708,13 +671,12 @@ namespace Server if (_pendingDelete.Remove(entity.Serial)) { Utility.PushColor(ConsoleColor.Red); - Console.WriteLine($"Deleted then added {typeof(T).Name} during {WorldState.ToString().ToLower()} state."); + WriteConsoleLine($"Deleted then added {typeof(T).Name} during {WorldState.ToString().ToLower()} state."); Utility.PopColor(); } _pendingAdd[entity.Serial] = entity; break; } - case WorldState.WritingLoadBuffers: case WorldState.Running: { if (entity.Serial.IsItem) @@ -753,7 +715,6 @@ namespace Server _pendingDelete[entity.Serial] = entity; break; } - case WorldState.WritingLoadBuffers: case WorldState.Running: { if (entity.Serial.IsItem) @@ -780,5 +741,17 @@ namespace Server // Resize to exact buffer size entity.SaveBuffer.Resize((int)entity.SaveBuffer.Position); } + + private static void WriteConsole(string message) + { + var now = DateTime.UtcNow; + Console.Write("[{0} {1}] World: {2}", now.ToShortDateString(), now.ToLongTimeString(), message); + } + + private static void WriteConsoleLine(string message) + { + var now = DateTime.UtcNow; + Console.WriteLine("[{0} {1}] World: {2}", now.ToShortDateString(), now.ToLongTimeString(), message); + } } }