-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Platform specific prefetcher #396
base: main
Are you sure you want to change the base?
Changes from 6 commits
8bc90be
b213951
a4630e0
43a9879
1cbb1fb
d1cf117
0465385
2d766b1
e2b275a
8748cea
a102ae6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
using System.ComponentModel; | ||
using System.Diagnostics; | ||
using System.Runtime.CompilerServices; | ||
using System.Runtime.InteropServices; | ||
|
||
namespace Paprika; | ||
|
||
public static class Platform | ||
{ | ||
public static void Prefetch(ReadOnlySpan<UIntPtr> addresses, UIntPtr size) => Manager.SchedulePrefetch(addresses, size); | ||
|
||
private static readonly IMemoryManager Manager = | ||
IsPosix() ? new PosixMemoryManager() : new WindowsMemoryManager(); | ||
|
||
private static bool IsPosix() => RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || | ||
RuntimeInformation.IsOSPlatform(OSPlatform.OSX); | ||
|
||
private sealed class PosixMemoryManager : IMemoryManager | ||
{ | ||
[Flags] | ||
private enum Advice : int | ||
{ | ||
// ReSharper disable InconsistentNaming | ||
/// <summary> | ||
/// Expect access in the near future. (Hence, it might be a good idea to read some pages ahead.) | ||
/// </summary> | ||
MADV_WILLNEED = 0x3, | ||
|
||
/// <summary> | ||
/// Do not expect access in the near future. | ||
/// (For the time being, the application is finished with the given range, | ||
/// so the kernel can free resources associated with it.) | ||
/// </summary> | ||
MADV_DONTNEED = 0x4, | ||
// ReSharper restore InconsistentNaming | ||
} | ||
|
||
[DllImport("LIBC_6", SetLastError = true)] | ||
static extern int madvise(IntPtr addr, UIntPtr length, Advice advice); | ||
|
||
public void SchedulePrefetch(ReadOnlySpan<UIntPtr> addresses, UIntPtr length) | ||
{ | ||
// TODO: | ||
} | ||
} | ||
|
||
private sealed class WindowsMemoryManager : IMemoryManager | ||
{ | ||
[DllImport("kernel32.dll", SetLastError = true)] | ||
private static extern unsafe bool PrefetchVirtualMemory(IntPtr hProcess, ulong numberOfEntries, | ||
Win32MemoryRangeEntry* entries, int flags); | ||
|
||
[StructLayout(LayoutKind.Sequential)] | ||
private struct Win32MemoryRangeEntry | ||
{ | ||
/// <summary> | ||
/// Starting address of the memory range | ||
/// </summary> | ||
public IntPtr VirtualAddress; | ||
|
||
/// <summary> | ||
/// Size of the memory range in bytes | ||
/// </summary> | ||
public UIntPtr NumberOfBytes; | ||
} | ||
|
||
[SkipLocalsInit] | ||
public unsafe void SchedulePrefetch(ReadOnlySpan<UIntPtr> addresses, UIntPtr length) | ||
{ | ||
var count = addresses.Length; | ||
var ptr = stackalloc Win32MemoryRangeEntry[count]; | ||
var span = new Span<Win32MemoryRangeEntry>(ptr, count); | ||
|
||
for (var i = 0; i < span.Length; i++) | ||
{ | ||
span[i].VirtualAddress = (IntPtr)addresses[i]; | ||
span[i].NumberOfBytes = length; | ||
} | ||
|
||
const int reserved = 0; | ||
|
||
if (PrefetchVirtualMemory(Process.GetCurrentProcess().Handle, (ulong)count, ptr, reserved) == false) | ||
{ | ||
throw new Win32Exception(Marshal.GetLastWin32Error()); | ||
} | ||
} | ||
} | ||
|
||
private interface IMemoryManager | ||
{ | ||
/// <summary> | ||
/// Schedules an OS dependent prefetch. | ||
/// </summary> | ||
void SchedulePrefetch(ReadOnlySpan<UIntPtr> addresses, UIntPtr length); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,10 @@ | |
using System.Diagnostics; | ||
using System.Diagnostics.Metrics; | ||
using System.IO.MemoryMappedFiles; | ||
using System.Runtime.CompilerServices; | ||
using System.Runtime.InteropServices; | ||
using System.Threading.Channels; | ||
using System.Xml.Serialization; | ||
using Paprika.Utils; | ||
|
||
namespace Paprika.Store.PageManagers; | ||
|
@@ -24,6 +27,18 @@ public sealed class MemoryMappedPageManager : PointerPageManager | |
private readonly MemoryMappedViewAccessor _whole; | ||
private readonly unsafe byte* _ptr; | ||
|
||
// Prefetcher | ||
private const int PrefetcherCapacity = 1000; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it better to make this a configurable parameter instead of a hardcoded value? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, could that configuration parameter be used to turn-off prefetching altogether in case it is not required? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure thing, will add such capability. |
||
private readonly Channel<DbAddress> _prefetches = Channel.CreateBounded<DbAddress>(new BoundedChannelOptions(PrefetcherCapacity) | ||
{ | ||
FullMode = BoundedChannelFullMode.DropOldest, | ||
SingleReader = true, | ||
SingleWriter = false, | ||
Capacity = PrefetcherCapacity, | ||
AllowSynchronousContinuations = false | ||
}); | ||
private readonly Task _prefetcher; | ||
|
||
// Flusher section | ||
private readonly Stack<PageMemoryOwner> _owners = new(); | ||
private readonly List<PageMemoryOwner> _ownersUsed = new(); | ||
|
@@ -72,6 +87,8 @@ public unsafe MemoryMappedPageManager(long size, byte historyDepth, string dir, | |
_meter = new Meter("Paprika.Store.PageManager"); | ||
_fileWrites = _meter.CreateHistogram<int>("File writes", "Syscall", "Actual numbers of file writes issued"); | ||
_writeTime = _meter.CreateHistogram<int>("Write time", "ms", "Time spent in writing"); | ||
|
||
_prefetcher = Task.Factory.StartNew(RunPrefetcher); | ||
} | ||
|
||
public static string GetPaprikaFilePath(string dir) => System.IO.Path.Combine(dir, PaprikaFileName); | ||
|
@@ -80,6 +97,51 @@ public unsafe MemoryMappedPageManager(long size, byte historyDepth, string dir, | |
|
||
protected override unsafe void* Ptr => _ptr; | ||
|
||
public override void Prefetch(ReadOnlySpan<DbAddress> addresses) | ||
{ | ||
var writer = _prefetches.Writer; | ||
|
||
foreach (var address in addresses) | ||
{ | ||
if (address.IsNull == false) | ||
{ | ||
writer.TryWrite(address); | ||
} | ||
} | ||
} | ||
|
||
private async Task RunPrefetcher() | ||
{ | ||
var reader = _prefetches.Reader; | ||
|
||
while (await reader.WaitToReadAsync()) | ||
{ | ||
PrefetchImpl(reader, this); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A basic question: Since the prefetcher runner thread is running asynchronously, is it theoretically possible that for a certain address it executes the prefetch after the actual read on that address is already done? Prefetcher will be able to find that the address is already in memory, but still these calls can effectively double the number of IOs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is possible that it will run after the page is fetched. And yes, it's possible that it will be fetching over pages that are already there. We could then discuss whether to leave SSE-based soft prefetch and leave this implementation for heavier use. The place that I'd still see it as is, would be the The next place that it works (just following the raw execution tiem) is visiting. We schedule prefetching for all the children up front and as it walks in the preorder, beside the first child the rest most likely will have time to be prefetched. Again, raw execution time shows a very nice boost (also, disk throughput is much higher) The one place that makes it less easy to reason about is the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for clarifying! Make sense to me to use two different variants based on the usage 😄 |
||
} | ||
|
||
[SkipLocalsInit] | ||
static void PrefetchImpl(ChannelReader<DbAddress> reader, MemoryMappedPageManager manager) | ||
{ | ||
const int maxPrefetch = 128; | ||
|
||
Span<UIntPtr> span = stackalloc UIntPtr[maxPrefetch]; | ||
var i = 0; | ||
|
||
for (; i < maxPrefetch; i++) | ||
{ | ||
if (reader.TryRead(out var address) == false) | ||
break; | ||
|
||
span[i] = manager.GetAt(address).Raw; | ||
} | ||
|
||
if (i > 0) | ||
{ | ||
Platform.Prefetch(span[..i], Page.PageSize); | ||
} | ||
} | ||
} | ||
|
||
public override async ValueTask FlushPages(ICollection<DbAddress> dbAddresses, CommitOptions options) | ||
{ | ||
if (_options == PersistenceOptions.MMapOnly) | ||
|
@@ -182,6 +244,9 @@ public override void ForceFlush() | |
|
||
public override void Dispose() | ||
{ | ||
_prefetches.Writer.Complete(); | ||
_prefetcher.Wait(); | ||
|
||
_meter.Dispose(); | ||
|
||
_whole.SafeMemoryMappedViewHandle.ReleasePointer(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New to Github PR reviews, can't find a way to comment on the existing line of code.
But anyway, the comment above describing that "prefetching is futile" needs to updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, it could be made more specific as this is about the
Sequential
flag as we don't doSequential
because clearly it's a random access all over the place. Will amend.