From c9ad39fd8da66c6b0355f29a14934dfea92a704e Mon Sep 17 00:00:00 2001 From: Thomas Gibson-Robinson Date: Fri, 22 Aug 2025 11:36:46 +0000 Subject: [PATCH] Fix a windows performance issue when shutting down block devices Whilst writing the integration test for buildbarn in bb-deployments, I noticed that bb_storage took a long time to shutdown on Windows. When looking at the system monitor, bb_storage was saturating the disk writing out data on shutdown. This was unexpected, since the test only wrote a small amount of data to the cache, so only a small amount of the block device should have been used (hundreds of MBs out of the GBs in the block device). It seems that when using SetEndOfFile, Windows will write out the entire file's contents when the process gets shutdown. This is a problem if buildbarn is started with a large block device but only uses a small part of it, like in the deployment tests. Following the suggestions at https://devblogs.microsoft.com/oldnewthing/20110922-00/?p=9573 we now create a sparse file and instantly zero the entire file's contents (if it's new or is resized). This completely fixes the performance issues. --- .../new_block_device_from_file_windows.go | 84 +++++++++++++++++-- 1 file changed, 79 insertions(+), 5 deletions(-) diff --git a/pkg/blockdevice/new_block_device_from_file_windows.go b/pkg/blockdevice/new_block_device_from_file_windows.go index 63f4432e..51b1c2ee 100644 --- a/pkg/blockdevice/new_block_device_from_file_windows.go +++ b/pkg/blockdevice/new_block_device_from_file_windows.go @@ -50,6 +50,15 @@ func NewBlockDeviceFromFile(path string, minimumSizeBytes int, zeroInitialize bo return nil, 0, 0, util.StatusWrapf(err, "Failed to open file %#v", path) } + bd, sectorSizeBytes, sectorCount, err := createBlockDevice(path, minimumSizeBytes, zeroInitialize, handle) + if err != nil { + windows.CloseHandle(handle) + return nil, 0, 0, err + } + return bd, sectorSizeBytes, sectorCount, nil +} + +func createBlockDevice(path string, minimumSizeBytes int, zeroInitialize bool, handle windows.Handle) (BlockDevice, int, int64, error) { // Get the sector size from the disk where the file is located. // Extract the root path (drive letter) from the full path. @@ -59,7 +68,6 @@ func NewBlockDeviceFromFile(path string, minimumSizeBytes int, zeroInitialize bo } rootPathPtr, err := syscall.UTF16PtrFromString(rootPath) if err != nil { - windows.CloseHandle(handle) return nil, 0, 0, util.StatusWrapf(err, "Failed to convert root path %#v to UTF-16", rootPath) } @@ -72,7 +80,6 @@ func NewBlockDeviceFromFile(path string, minimumSizeBytes int, zeroInitialize bo /*numberOfFreeClusters=*/ 0, /*totalNumberOfClusters=*/ 0) if r == 0 { - windows.CloseHandle(handle) return nil, 0, 0, util.StatusWrapf(err, "Failed to get disk sector size for %#v", rootPath) } @@ -82,25 +89,92 @@ func NewBlockDeviceFromFile(path string, minimumSizeBytes int, zeroInitialize bo sectorCount := int64((uint64(minimumSizeBytes) + uint64(sectorSizeBytes) - 1) / uint64(sectorSizeBytes)) sizeBytes := int64(sectorSizeBytes) * sectorCount + // Determine if we need to make the file sparse. + // If we want to zero initialise the file, then we always make the + // file sparse as this is the most efficient way of doing so. + // We also mark the file as sparse if we need to resize the file. + // If we do not mark the file as sparse, then when this process + // terminates, Windows will write out the entire file's contents + // to disk, even if only 1 byte has been written to the file. See + // https://devblogs.microsoft.com/oldnewthing/20110922-00/?p=9573 + // for further details. + var makeFileSparse bool + if zeroInitialize { + makeFileSparse = true + } else { + // Check if we need to resize the file. + var fileInfo windows.ByHandleFileInformation + if err := windows.GetFileInformationByHandle(handle, &fileInfo); err != nil { + return nil, 0, 0, util.StatusWrapf(err, "Failed to get file information for %#v", path) + } + existingSize := int64(fileInfo.FileSizeHigh)<<32 | int64(fileInfo.FileSizeLow) + makeFileSparse = existingSize < int64(minimumSizeBytes) + } + var fileSizeHigh int32 = int32(sizeBytes >> 32) if _, err = windows.SetFilePointer(handle, int32(sizeBytes), (*int32)(unsafe.Pointer(&fileSizeHigh)), windows.FILE_BEGIN); err != nil { - windows.CloseHandle(handle) return nil, 0, 0, util.StatusWrapf(err, "Failed to set file pointer for %#v", path) } if err := windows.SetEndOfFile(handle); err != nil { - windows.CloseHandle(handle) return nil, 0, 0, util.StatusWrapf(err, "Failed to set end of file for %#v to %d bytes", path, sizeBytes) } + if makeFileSparse { + if err := makeSparseFile(handle, sizeBytes); err != nil { + return nil, 0, 0, util.StatusWrapf(err, "Failed to make file sparse for %#v", path) + } + } + bd, err := newMemoryMappedBlockDevice(handle, int(sizeBytes)) if err != nil { - windows.CloseHandle(handle) return nil, 0, 0, err } return bd, sectorSizeBytes, sectorCount, nil } +func makeSparseFile(handle windows.Handle, sizeBytes int64) error { + // FILE_ZERO_DATA_INFORMATION + type fileZeroDataInformation struct { + FileOffset int64 + BeyondFinalZero int64 + } + + // Mark the file as sparse. + var bytesReturned uint32 + if err := windows.DeviceIoControl( + handle, + windows.FSCTL_SET_SPARSE, + /*inBuffer=*/ nil, + /*inBufferSize=*/ 0, + /*outBuffer=*/ nil, + /*outBufferSize=*/ 0, + &bytesReturned, + /*overlapped=*/ nil, + ); err != nil { + return util.StatusWrapf(err, "Failed to mark file as sparse") + } + // Zero the file's data. + zeroData := fileZeroDataInformation{ + FileOffset: 0, + BeyondFinalZero: sizeBytes, + } + if err := windows.DeviceIoControl( + handle, + windows.FSCTL_SET_ZERO_DATA, + (*byte)(unsafe.Pointer(&zeroData)), + uint32(unsafe.Sizeof(zeroData)), + /*outBuffer=*/ nil, + /*outBufferSize=*/ 0, + &bytesReturned, + /*overlapped=*/ nil, + ); err != nil { + return util.StatusWrapf(err, "Failed to zero sparse file data") + } + + return nil +} + // A ScopeWalker that extracts the root path for GetDiskFreeSpaceW. type rootPathScopeWalker struct { rootPath string