From 7527155cb87c382cab1848b5bf65910c2dead291 Mon Sep 17 00:00:00 2001 From: Bill Zissimopoulos Date: Tue, 22 Mar 2022 16:47:40 +0000 Subject: [PATCH] dll: dirbuf: - FspFileSystemAcquireDirectoryBufferEx takes hint for initial capacity. - Buffer allocation strategy has been improved to minimize reallocation. - Quick sort of directory entries now implements median of three partitioning. This improves performance of sorting already sorted data. --- inc/winfsp/winfsp.h | 2 ++ src/dll/dirbuf.c | 38 ++++++++++++++++---- tst/ntptfs/ptfs.c | 18 +++++++++- tst/winfsp-tests/dirbuf-test.c | 64 +++++++++++++++++++++++++++------- 4 files changed, 102 insertions(+), 20 deletions(-) diff --git a/inc/winfsp/winfsp.h b/inc/winfsp/winfsp.h index ba753a6e..499c9b8b 100644 --- a/inc/winfsp/winfsp.h +++ b/inc/winfsp/winfsp.h @@ -1750,6 +1750,8 @@ FSP_API BOOLEAN FspFileSystemAddNotifyInfo(FSP_FSCTL_NOTIFY_INFO *NotifyInfo, /* * Directory buffering */ +FSP_API BOOLEAN FspFileSystemAcquireDirectoryBufferEx(PVOID* PDirBuffer, + BOOLEAN Reset, ULONG CapacityHint, PNTSTATUS PResult); FSP_API BOOLEAN FspFileSystemAcquireDirectoryBuffer(PVOID *PDirBuffer, BOOLEAN Reset, PNTSTATUS PResult); FSP_API BOOLEAN FspFileSystemFillDirectoryBuffer(PVOID *PDirBuffer, diff --git a/src/dll/dirbuf.c b/src/dll/dirbuf.c index bde3b98d..3910835f 100644 --- a/src/dll/dirbuf.c +++ b/src/dll/dirbuf.c @@ -21,6 +21,11 @@ #include +#define FspFileSystemDirectoryBufferLoBound (256) +#define FspFileSystemDirectoryBufferHiBound (1024 * 1024) +#define FspFileSystemDirectoryBufferLoFactor (4) +#define FspFileSystemDirectoryBufferHiFactor (2) + #define RETURN(R, B) \ do \ { \ @@ -32,7 +37,7 @@ typedef struct { SRWLOCK Lock; - ULONG Capacity, LoMark, HiMark; + ULONG InitialCapacity, Capacity, LoMark, HiMark; PUINT8 Buffer; } FSP_FILE_SYSTEM_DIRECTORY_BUFFER; @@ -179,12 +184,15 @@ static VOID FspFileSystemQSortDirectoryBuffer(PUINT8 Buffer, PULONG Index, int l { while (r > l) { -#if 0 +#if 1 exch(Index[(l + r) / 2], Index[r - 1]); compexch(Index[l], Index[r - 1]); compexch(Index[l], Index[r]); compexch(Index[r - 1], Index[r]); + if (r - 1 <= l + 1) + break; + i = FspFileSystemPartitionDirectoryBuffer(Buffer, Index, l + 1, r - 1); #else i = FspFileSystemPartitionDirectoryBuffer(Buffer, Index, l, r); @@ -224,8 +232,8 @@ static inline VOID FspFileSystemSortDirectoryBuffer(FSP_FILE_SYSTEM_DIRECTORY_BU FspFileSystemQSortDirectoryBuffer(Buffer, Index, 0, Count - 1); } -FSP_API BOOLEAN FspFileSystemAcquireDirectoryBuffer(PVOID *PDirBuffer, - BOOLEAN Reset, PNTSTATUS PResult) +FSP_API BOOLEAN FspFileSystemAcquireDirectoryBufferEx(PVOID* PDirBuffer, + BOOLEAN Reset, ULONG CapacityHint, PNTSTATUS PResult) { FSP_FILE_SYSTEM_DIRECTORY_BUFFER *DirBuffer = FspInterlockedLoadPointer(PDirBuffer); @@ -234,10 +242,20 @@ FSP_API BOOLEAN FspFileSystemAcquireDirectoryBuffer(PVOID *PDirBuffer, static SRWLOCK CreateLock = SRWLOCK_INIT; FSP_FILE_SYSTEM_DIRECTORY_BUFFER *NewDirBuffer; + /* compute next (or equal) power of 2; ensure fits within bounds */ + ULONG bitidx; + if (_BitScanReverse(&bitidx, CapacityHint - 1)) + CapacityHint = (1 << (1 + bitidx)); + CapacityHint = FspFileSystemDirectoryBufferLoBound > CapacityHint ? + FspFileSystemDirectoryBufferLoBound : CapacityHint; + CapacityHint = FspFileSystemDirectoryBufferHiBound < CapacityHint ? + FspFileSystemDirectoryBufferHiBound : CapacityHint; + NewDirBuffer = MemAlloc(sizeof *NewDirBuffer); if (0 == NewDirBuffer) RETURN(STATUS_INSUFFICIENT_RESOURCES, FALSE); memset(NewDirBuffer, 0, sizeof *NewDirBuffer); + NewDirBuffer->InitialCapacity = CapacityHint; InitializeSRWLock(&NewDirBuffer->Lock); AcquireSRWLockExclusive(&NewDirBuffer->Lock); @@ -267,6 +285,12 @@ FSP_API BOOLEAN FspFileSystemAcquireDirectoryBuffer(PVOID *PDirBuffer, RETURN(STATUS_SUCCESS, FALSE); } +FSP_API BOOLEAN FspFileSystemAcquireDirectoryBuffer(PVOID *PDirBuffer, + BOOLEAN Reset, PNTSTATUS PResult) +{ + return FspFileSystemAcquireDirectoryBufferEx(PDirBuffer, Reset, 0, PResult); +} + FSP_API BOOLEAN FspFileSystemFillDirectoryBuffer(PVOID *PDirBuffer, FSP_FSCTL_DIR_INFO *DirInfo, PNTSTATUS PResult) { @@ -301,7 +325,7 @@ FSP_API BOOLEAN FspFileSystemFillDirectoryBuffer(PVOID *PDirBuffer, if (0 == Buffer) { - Buffer = MemAlloc(Capacity = 512); + Buffer = MemAlloc(Capacity = DirBuffer->InitialCapacity); if (0 == Buffer) RETURN(STATUS_INSUFFICIENT_RESOURCES, FALSE); @@ -309,7 +333,9 @@ FSP_API BOOLEAN FspFileSystemFillDirectoryBuffer(PVOID *PDirBuffer, } else { - Buffer = MemRealloc(Buffer, Capacity = DirBuffer->Capacity * 2); + ULONG Factor = FspFileSystemDirectoryBufferHiBound > DirBuffer->Capacity ? + FspFileSystemDirectoryBufferLoFactor : FspFileSystemDirectoryBufferHiFactor; + Buffer = MemRealloc(Buffer, Capacity = DirBuffer->Capacity * Factor); if (0 == Buffer) RETURN(STATUS_INSUFFICIENT_RESOURCES, FALSE); diff --git a/tst/ntptfs/ptfs.c b/tst/ntptfs/ptfs.c index 0edfeafb..37556e61 100644 --- a/tst/ntptfs/ptfs.c +++ b/tst/ntptfs/ptfs.c @@ -24,12 +24,14 @@ #define FileSystemContext ((PTFS *)(FileSystem)->UserContext) #define FileContextHandle (((FILE_CONTEXT *)(FileContext))->Handle) #define FileContextIsDirectory (((FILE_CONTEXT *)(FileContext))->IsDirectory) +#define FileContextDirFileSize (((FILE_CONTEXT *)(FileContext))->DirFileSize) #define FileContextDirBuffer (&((FILE_CONTEXT *)(FileContext))->DirBuffer) typedef struct { HANDLE Handle; BOOLEAN IsDirectory; + ULONG DirFileSize; PVOID DirBuffer; } FILE_CONTEXT; @@ -241,6 +243,7 @@ static NTSTATUS CreateEx(FSP_FILE_SYSTEM *FileSystem, memset(FileContext, 0, sizeof *FileContext); FileContext->Handle = Handle; FileContext->IsDirectory = IsDirectory; + FileContext->DirFileSize = (ULONG)FileInfo->FileSize; *PFileContext = FileContext; Result = STATUS_SUCCESS; @@ -331,6 +334,7 @@ static NTSTATUS Open(FSP_FILE_SYSTEM *FileSystem, memset(FileContext, 0, sizeof *FileContext); FileContext->Handle = Handle; FileContext->IsDirectory = IsDirectory; + FileContext->DirFileSize = (ULONG)FileInfo->FileSize; *PFileContext = FileContext; Result = STATUS_SUCCESS; @@ -787,6 +791,18 @@ static NTSTATUS BufferedReadDirectory(FSP_FILE_SYSTEM *FileSystem, { PTFS *Ptfs = FileSystemContext; HANDLE Handle = FileContextHandle; + ULONG CapacityHint = FileContextDirFileSize; + /* + * An analysis of the relationship between the required buffer capacity and the + * NTFS directory size (as reported by FileStandardInformation) showed the ratio + * between the two to be between 0.5 and 1 with some outliers outside those bounds. + * (For this analysis file names of average length of 4 or 24 were used and NTFS + * had 8.3 file names disabled.) + * + * We use the NTFS directory size as our capacity hint (i.e. ratio of 1), which + * means that we may overestimate the required buffer capacity in most cases. + * This is ok since our goal is to improve performance. + */ PVOID PDirBuffer = FileContextDirBuffer; BOOLEAN RestartScan; ULONG BytesTransferred; @@ -801,7 +817,7 @@ static NTSTATUS BufferedReadDirectory(FSP_FILE_SYSTEM *FileSystem, NTSTATUS Result, DirBufferResult; DirBufferResult = STATUS_SUCCESS; - if (FspFileSystemAcquireDirectoryBuffer(PDirBuffer, 0 == Marker, &DirBufferResult)) + if (FspFileSystemAcquireDirectoryBufferEx(PDirBuffer, 0 == Marker, CapacityHint, &DirBufferResult)) { for (RestartScan = TRUE;; RestartScan = FALSE) { diff --git a/tst/winfsp-tests/dirbuf-test.c b/tst/winfsp-tests/dirbuf-test.c index 4635d08e..66cb1af0 100644 --- a/tst/winfsp-tests/dirbuf-test.c +++ b/tst/winfsp-tests/dirbuf-test.c @@ -21,6 +21,7 @@ #include #include +#include #include #include "winfsp-tests.h" @@ -145,7 +146,7 @@ static void dirbuf_dots_test(void) FspFileSystemDeleteDirectoryBuffer(&DirBuffer); } -static void dirbuf_fill_dotest(unsigned seed, ULONG Count, ULONG InvalidCount) +static void dirbuf_fill_dotest(unsigned seed, ULONG Count, ULONG InvalidCount, BOOLEAN Presort) { PVOID DirBuffer = 0; NTSTATUS Result; @@ -162,6 +163,7 @@ static void dirbuf_fill_dotest(unsigned seed, ULONG Count, ULONG InvalidCount) ULONG DotIndex = Count / 3, DotDotIndex = Count / 3 * 2; WCHAR Marker[MAX_PATH]; ULONG N, MarkerIndex, MarkerLength; + UINT64 PresortIndex = 0; srand(seed); @@ -189,12 +191,18 @@ static void dirbuf_fill_dotest(unsigned seed, ULONG Count, ULONG InvalidCount) DirInfo->FileNameBuf[0] = L'.'; DirInfo->FileNameBuf[1] = L'.'; } - else + else if (!Presort) { N = 24 + rand() % (32 - 24); for (ULONG J = 0; N > J; J++) DirInfo->FileNameBuf[J] = 'A' + rand() % 26; } + else + { + N = 24; + StringCbPrintfW(DirInfo->FileNameBuf, 64, L"FILEFILE%016llx", PresortIndex); + PresortIndex++; + } DirInfo->Size = (UINT16)(sizeof(FSP_FSCTL_DIR_INFO) + N * sizeof(WCHAR)); Success = FspFileSystemFillDirectoryBuffer(&DirBuffer, DirInfo, &Result); @@ -212,7 +220,7 @@ static void dirbuf_fill_dotest(unsigned seed, ULONG Count, ULONG InvalidCount) typedef struct { SRWLOCK Lock; - ULONG Capacity, LoMark, HiMark; + ULONG InitialCapacity, Capacity, LoMark, HiMark; PUINT8 Buffer; } FSP_FILE_SYSTEM_DIRECTORY_BUFFER; FSP_FILE_SYSTEM_DIRECTORY_BUFFER *PeekDirBuffer = DirBuffer; @@ -348,31 +356,60 @@ static void dirbuf_fill_test(void) { unsigned seed = (unsigned)time(0); - dirbuf_fill_dotest(1485473509, 10, 0); - dirbuf_fill_dotest(1485473509, 10, 3); + dirbuf_fill_dotest(1485473509, 10, 0, FALSE); + dirbuf_fill_dotest(1485473509, 10, 3, FALSE); for (ULONG I = 0; 10000 > I; I++) { - dirbuf_fill_dotest(seed + I, 10, 0); - dirbuf_fill_dotest(seed + I, 10, 3); + dirbuf_fill_dotest(seed + I, 10, 0, FALSE); + dirbuf_fill_dotest(seed + I, 10, 3, FALSE); } for (ULONG I = 0; 1000 > I; I++) { - dirbuf_fill_dotest(seed + I, 100, 0); - dirbuf_fill_dotest(seed + I, 100, 30); + dirbuf_fill_dotest(seed + I, 100, 0, FALSE); + dirbuf_fill_dotest(seed + I, 100, 30, FALSE); } for (ULONG I = 0; 100 > I; I++) { - dirbuf_fill_dotest(seed + I, 1000, 0); - dirbuf_fill_dotest(seed + I, 1000, 300); + dirbuf_fill_dotest(seed + I, 1000, 0, FALSE); + dirbuf_fill_dotest(seed + I, 1000, 300, FALSE); } for (ULONG I = 0; 10 > I; I++) { - dirbuf_fill_dotest(seed + I, 10000, 0); - dirbuf_fill_dotest(seed + I, 10000, 3000); + dirbuf_fill_dotest(seed + I, 10000, 0, FALSE); + dirbuf_fill_dotest(seed + I, 10000, 3000, FALSE); + } +} + +static void dirbuf_presort_fill_test(void) +{ + unsigned seed = (unsigned)time(0); + + for (ULONG I = 0; 10000 > I; I++) + { + dirbuf_fill_dotest(seed + I, 10, 0, TRUE); + dirbuf_fill_dotest(seed + I, 10, 3, TRUE); + } + + for (ULONG I = 0; 1000 > I; I++) + { + dirbuf_fill_dotest(seed + I, 100, 0, TRUE); + dirbuf_fill_dotest(seed + I, 100, 30, TRUE); + } + + for (ULONG I = 0; 100 > I; I++) + { + dirbuf_fill_dotest(seed + I, 1000, 0, TRUE); + dirbuf_fill_dotest(seed + I, 1000, 300, TRUE); + } + + for (ULONG I = 0; 10 > I; I++) + { + dirbuf_fill_dotest(seed + I, 10000, 0, FALSE); + dirbuf_fill_dotest(seed + I, 10000, 3000, FALSE); } } @@ -472,5 +509,6 @@ void dirbuf_tests(void) TEST(dirbuf_empty_test); TEST(dirbuf_dots_test); TEST(dirbuf_fill_test); + TEST(dirbuf_presort_fill_test); TEST(dirbuf_boundary_test); }