From 34634ec357a9fc58722eabc6d790568d1cd7ea3d Mon Sep 17 00:00:00 2001 From: Nikolay Date: Sat, 25 Mar 2023 19:43:19 +0100 Subject: [PATCH] lib/fs: adds memory map for windows (#3988) This is a follow-up for 43b24164efdd35c2641a289ab93f5cb94279cb60 * lib/fs: adds memory map for windows it should improve performance for file reading * lib/storage: replace '/' with os specific separator it must fix an errors for windows * lib/fs: mention windows fsync support * lib/filestream: adds fdatasync for windows writes Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/70 --- lib/filestream/filestream_windows.go | 11 +++ lib/fs/fs_windows.go | 114 ++++++++++++++------------- lib/storage/part_header.go | 5 +- lib/storage/partition.go | 7 +- lib/storage/table.go | 4 +- 5 files changed, 76 insertions(+), 65 deletions(-) diff --git a/lib/filestream/filestream_windows.go b/lib/filestream/filestream_windows.go index 040660083..a46c935a9 100644 --- a/lib/filestream/filestream_windows.go +++ b/lib/filestream/filestream_windows.go @@ -1,6 +1,17 @@ package filestream +import ( + "fmt" + + "golang.org/x/sys/windows" +) + func (st *streamTracker) adviseDontNeed(n int, fdatasync bool) error { + if fdatasync && st.fd > 0 { + if err := windows.Fsync(windows.Handle(st.fd)); err != nil { + return fmt.Errorf("windows.Fsync error: %w", err) + } + } return nil } diff --git a/lib/fs/fs_windows.go b/lib/fs/fs_windows.go index a64748cc2..27f8a3cd4 100644 --- a/lib/fs/fs_windows.go +++ b/lib/fs/fs_windows.go @@ -3,6 +3,8 @@ package fs import ( "fmt" "os" + "reflect" + "sync" "unsafe" "github.com/VictoriaMetrics/VictoriaMetrics/lib/logger" @@ -10,27 +12,20 @@ import ( ) var ( - kernelDLL = windows.MustLoadDLL("kernel32.dll") - procLock = kernelDLL.MustFindProc("LockFileEx") - procEvent = kernelDLL.MustFindProc("CreateEventW") - procDisk = kernelDLL.MustFindProc("GetDiskFreeSpaceExW") - ntDLL = windows.MustLoadDLL("ntdll.dll") - ntSetInformationProc = ntDLL.MustFindProc("NtSetInformationFile") + kernelDLL = windows.MustLoadDLL("kernel32.dll") + procLock = kernelDLL.MustFindProc("LockFileEx") + procEvent = kernelDLL.MustFindProc("CreateEventW") + procDisk = kernelDLL.MustFindProc("GetDiskFreeSpaceExW") ) -// panic at windows, if file already open by another process. -// one of possible solutions - change files opening process with correct flags. -// https://github.com/dgraph-io/badger/issues/699 -// https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-flushfilebuffers -func mustSyncPath(string) { +// at windows only files could be synced +// Sync for directories is not supported. +func mustSyncPath(path string) { } const ( lockfileExclusiveLock = 2 fileFlagNormal = 0x00000080 - // https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/ntddk/ns-ntddk-_file_disposition_information_ex - fileDispositionPosixSemantics = 0x00000002 - fileDispositionIgnoreReadonlyAttribute = 0x00000010 ) // https://github.com/juju/fslock/blob/master/fslock_windows.go @@ -55,7 +50,6 @@ func createFlockFile(flockFile string) (*os.File, error) { return nil, fmt.Errorf("cannot create Overlapped handler: %w", err) } // https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-lockfileex - // overlapped is dropped? r1, _, err := procLock.Call(uintptr(handle), uintptr(lockfileExclusiveLock), uintptr(0), uintptr(1), uintptr(0), uintptr(unsafe.Pointer(ol))) if r1 == 0 { return nil, err @@ -63,14 +57,57 @@ func createFlockFile(flockFile string) (*os.File, error) { return os.NewFile(uintptr(handle), flockFile), nil } -// stub +var ( + fileMappingMU sync.Mutex + fileMappingByAddr = map[uintptr]windows.Handle{} +) + func mmap(fd int, length int) ([]byte, error) { - return nil, nil + flProtect := uint32(windows.PAGE_READONLY) + dwDesiredAccess := uint32(windows.FILE_MAP_READ) + // https://learn.microsoft.com/en-us/windows/win32/memory/creating-a-file-mapping-object#file-mapping-size + // do not specify any length params, windows will set it according to the file size. + // If length > file size, truncate is required according to api definition, we don't want it. + h, errno := windows.CreateFileMapping(windows.Handle(fd), nil, flProtect, 0, 0, nil) + if h == 0 { + return nil, os.NewSyscallError("CreateFileMapping", errno) + } + addr, errno := windows.MapViewOfFile(h, dwDesiredAccess, 0, 0, 0) + if addr == 0 { + windows.CloseHandle(h) + return nil, os.NewSyscallError("MapViewOfFile", errno) + } + fileMappingMU.Lock() + fileMappingByAddr[addr] = h + fileMappingMU.Unlock() + data := make([]byte, 0) + hdr := (*reflect.SliceHeader)(unsafe.Pointer(&data)) + hdr.Data = addr + hdr.Len = length + hdr.Cap = hdr.Len + + return data, nil } -// stub -func mUnmap([]byte) error { - return nil +func mUnmap(data []byte) error { + // flush is not needed, since we perform only reading operation. + // In case of write, additional call FlushViewOfFile must be performed. + header := (*reflect.SliceHeader)(unsafe.Pointer(&data)) + addr := header.Data + fileMappingMU.Lock() + defer fileMappingMU.Unlock() + if err := windows.UnmapViewOfFile(addr); err != nil { + return err + } + + handle, ok := fileMappingByAddr[addr] + if !ok { + logger.Fatalf("BUG: unmapping for non exist addr: %d", addr) + } + delete(fileMappingByAddr, addr) + + e := windows.CloseHandle(handle) + return os.NewSyscallError("CloseHandle", e) } func mustGetFreeSpace(path string) uint64 { @@ -78,7 +115,7 @@ func mustGetFreeSpace(path string) uint64 { r, _, err := procDisk.Call(uintptr(unsafe.Pointer(windows.StringToUTF16Ptr(path))), uintptr(unsafe.Pointer(&freeBytes))) if r == 0 { - logger.Errorf("cannot get free space: %v", err) + logger.Errorf("cannot get free space for path: %q : %s", path, err) return 0 } return uint64(freeBytes) @@ -109,38 +146,3 @@ func createEvent(sa *windows.SecurityAttributes, name *uint16) (windows.Handle, } return handle, nil } - -// https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/ntddk/ns-ntddk-_file_disposition_information_ex -type fileDispositionInformationEx struct { - Flags uint32 -} - -// https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/ns-wdm-_io_status_block -type ioStatusBlock struct { - Status, Information uintptr -} - -// UpdateFileHandle - changes file deletion semantic at windows to posix-like. -func UpdateFileHandle(path string) error { - handle, err := windows.Open(path, windows.GENERIC_READ|windows.DELETE, windows.FILE_SHARE_READ|windows.FILE_SHARE_DELETE) - if err != nil { - return err - } - return setPosixDelete(handle) -} - -// supported starting with Windows 10, version 1709. -// supported by NTFS only. -func setPosixDelete(handle windows.Handle) error { - var iosb ioStatusBlock - flags := fileDispositionInformationEx{ - Flags: fileDispositionPosixSemantics | fileDispositionIgnoreReadonlyAttribute, - } - // class FileDispositionInformationEx, // 64 - // https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/ne-wdm-_file_information_class - r0, _, err := ntSetInformationProc.Call(uintptr(handle), uintptr(unsafe.Pointer(&iosb)), uintptr(unsafe.Pointer(&flags)), unsafe.Sizeof(flags), uintptr(64)) - if r0 == 0 { - return nil - } - return fmt.Errorf("cannot set file disposition information: NT_STATUS: 0x%X, error: %w", r0, err) -} diff --git a/lib/storage/part_header.go b/lib/storage/part_header.go index ee61724fb..5d1412f98 100644 --- a/lib/storage/part_header.go +++ b/lib/storage/part_header.go @@ -83,11 +83,10 @@ func (ph *partHeader) ParseFromPath(path string) error { path = filepath.Clean(path) // Extract encoded part name. - n := strings.LastIndexByte(path, '/') - if n < 0 { + dir, partName := filepath.Split(path) + if len(dir) == 0 { return fmt.Errorf("cannot find encoded part name in the path %q", path) } - partName := path[n+1:] // PartName must have the following form: // RowsCount_BlocksCount_MinTimestamp_MaxTimestamp_Garbage diff --git a/lib/storage/partition.go b/lib/storage/partition.go index f9919f616..c25959e2c 100644 --- a/lib/storage/partition.go +++ b/lib/storage/partition.go @@ -259,13 +259,12 @@ func openPartition(smallPartsPath, bigPartsPath string, s *Storage) (*partition, smallPartsPath = filepath.Clean(smallPartsPath) bigPartsPath = filepath.Clean(bigPartsPath) - n := strings.LastIndexByte(smallPartsPath, '/') - if n < 0 { + dir, name := filepath.Split(smallPartsPath) + if len(dir) == 0 { return nil, fmt.Errorf("cannot find partition name from smallPartsPath %q; must be in the form /path/to/smallparts/YYYY_MM", smallPartsPath) } - name := smallPartsPath[n+1:] - if !strings.HasSuffix(bigPartsPath, "/"+name) { + if !strings.HasSuffix(bigPartsPath, name) { return nil, fmt.Errorf("patititon name in bigPartsPath %q doesn't match smallPartsPath %q; want %q", bigPartsPath, smallPartsPath, name) } diff --git a/lib/storage/table.go b/lib/storage/table.go index 95fd3a657..870185017 100644 --- a/lib/storage/table.go +++ b/lib/storage/table.go @@ -522,8 +522,8 @@ func openPartitions(smallPartitionsPath, bigPartitionsPath string, s *Storage) ( } var pts []*partition for ptName := range ptNames { - smallPartsPath := smallPartitionsPath + "/" + ptName - bigPartsPath := bigPartitionsPath + "/" + ptName + smallPartsPath := filepath.Join(smallPartitionsPath, ptName) + bigPartsPath := filepath.Join(bigPartitionsPath, ptName) pt, err := openPartition(smallPartsPath, bigPartsPath, s) if err != nil { mustClosePartitions(pts)