diff --git a/.gitignore b/.gitignore index 769557ef3..4eac4703b 100644 --- a/.gitignore +++ b/.gitignore @@ -21,3 +21,4 @@ changelog-news.txt ReleaseBuilder/build-temp/ ReleaseBuilder/Resources/Windows/TrayIcon/binfiles.wxs ReleaseBuilder/Resources/Windows/Agent/binfiles.wxs +Tools/MacOSPhotosNative/build/ diff --git a/Duplicati/Library/Snapshots/MacOS/MacOSPhotosNative.cs b/Duplicati/Library/Snapshots/MacOS/MacOSPhotosNative.cs index e788a496c..dc991498c 100644 --- a/Duplicati/Library/Snapshots/MacOS/MacOSPhotosNative.cs +++ b/Duplicati/Library/Snapshots/MacOS/MacOSPhotosNative.cs @@ -156,27 +156,84 @@ internal static class MacOSPhotosNative if (buffer.Length == 0) return 0; - var errorPtr = IntPtr.Zero; - int bytesRead; - - // TODO: Optimize to read directly into the provided buffer - var tempbuffer = new byte[buffer.Length]; - var result = NativeMethods.DuplicatiPhotosReadAsset(handle, tempbuffer, (nuint)tempbuffer.Length, out errorPtr); - if (result < 0) + // A Span cannot be pinned via a GCHandle, so use a pooled rented array + // as a bounce buffer. Callers that hold an array or Memory use the + // allocation-free overload below. + var rented = System.Buffers.ArrayPool.Shared.Rent(buffer.Length); + try { - var message = ConsumeErrorMessage(ref errorPtr) ?? "Failed to read Photos asset data."; - throw new MacOSPhotosException(message); + var read = ReadIntoArray(handle, rented, 0, buffer.Length); + if (read > 0) + rented.AsSpan(0, read).CopyTo(buffer); + return read; } - tempbuffer.AsSpan(0, (int)result).CopyTo(buffer); - bytesRead = checked((int)result); - - if (errorPtr != IntPtr.Zero) + finally { - NativeMethods.DuplicatiPhotosFreeString(errorPtr); - errorPtr = IntPtr.Zero; + System.Buffers.ArrayPool.Shared.Return(rented); } + } - return bytesRead; + /// + /// Reads data from the asset into the provided buffer + /// + /// The asset handle + /// The buffer to read data into + /// The number of bytes read + internal static int ReadAsset(SafeAssetHandle handle, Memory buffer) + { + if (handle.IsInvalid) + throw new ObjectDisposedException(nameof(SafeAssetHandle)); + + if (buffer.Length == 0) + return 0; + + // Try to read directly into the caller-provided array by pinning it, + // avoiding an intermediate managed allocation on every read. If the + // buffer is not array-backed, fall back to the Span overload, which + // uses a pooled rented array. + if (MemoryMarshal.TryGetArray(buffer, out var segment) && segment.Array != null) + return ReadIntoArray(handle, segment.Array, segment.Offset, segment.Count); + + return ReadAsset(handle, buffer.Span); + } + + /// + /// Reads data into a pinned region of the provided array + /// + /// The asset handle + /// The backing array to read into + /// The offset within the array + /// The number of bytes to read + /// The number of bytes read + private static int ReadIntoArray(SafeAssetHandle handle, byte[] array, int offset, int count) + { + // Pin the array and pass the element address to the native reader. Using + // a pinned GCHandle keeps this allocation-free and does not require the + // project to enable unsafe code blocks. + var gch = GCHandle.Alloc(array, GCHandleType.Pinned); + try + { + var bufferPtr = Marshal.UnsafeAddrOfPinnedArrayElement(array, offset); + var result = NativeMethods.DuplicatiPhotosReadAsset(handle, bufferPtr, (nuint)count, out var errorPtr); + + if (result < 0) + { + var message = ConsumeErrorMessage(ref errorPtr) ?? "Failed to read Photos asset data."; + throw new MacOSPhotosException(message); + } + + if (errorPtr != IntPtr.Zero) + { + NativeMethods.DuplicatiPhotosFreeString(errorPtr); + errorPtr = IntPtr.Zero; + } + + return checked((int)result); + } + finally + { + gch.Free(); + } } /// @@ -355,7 +412,7 @@ internal static class MacOSPhotosNative /// The error message, if any /// The number of bytes read [DllImport(LibraryName, CallingConvention = CallingConvention.Cdecl)] - internal static extern nint DuplicatiPhotosReadAsset(SafeAssetHandle handle, byte[] buffer, nuint length, out IntPtr errorMessage); + internal static extern nint DuplicatiPhotosReadAsset(SafeAssetHandle handle, IntPtr buffer, nuint length, out IntPtr errorMessage); /// /// Closes the asset handle diff --git a/Duplicati/Library/Snapshots/MacOS/MacOSPhotosNativeStream.cs b/Duplicati/Library/Snapshots/MacOS/MacOSPhotosNativeStream.cs index 2c493db23..fca6e6cc5 100644 --- a/Duplicati/Library/Snapshots/MacOS/MacOSPhotosNativeStream.cs +++ b/Duplicati/Library/Snapshots/MacOS/MacOSPhotosNativeStream.cs @@ -71,7 +71,7 @@ internal sealed class MacOSPhotosNativeStream : Stream if (count == 0) return 0; - var bytesRead = MacOSPhotosNative.ReadAsset(handle, new Span(buffer, offset, count)); + var bytesRead = MacOSPhotosNative.ReadAsset(handle, new Memory(buffer, offset, count)); position += bytesRead; return bytesRead; } @@ -92,7 +92,7 @@ internal sealed class MacOSPhotosNativeStream : Stream if (disposed) throw new ObjectDisposedException(nameof(MacOSPhotosNativeStream)); - var bytesRead = MacOSPhotosNative.ReadAsset(handle, buffer.Span); + var bytesRead = MacOSPhotosNative.ReadAsset(handle, buffer); position += bytesRead; return ValueTask.FromResult(bytesRead); } @@ -108,7 +108,7 @@ internal sealed class MacOSPhotosNativeStream : Stream if (count == 0) return Task.FromResult(0); - var bytesRead = MacOSPhotosNative.ReadAsset(handle, new Span(buffer, offset, count)); + var bytesRead = MacOSPhotosNative.ReadAsset(handle, new Memory(buffer, offset, count)); position += bytesRead; return Task.FromResult(bytesRead); } diff --git a/Tools/MacOSPhotosNative/DuplicatiPhotos.m b/Tools/MacOSPhotosNative/DuplicatiPhotos.m index f70511977..f22727fe1 100644 --- a/Tools/MacOSPhotosNative/DuplicatiPhotos.m +++ b/Tools/MacOSPhotosNative/DuplicatiPhotos.m @@ -287,8 +287,22 @@ void DuplicatiPhotosFreeAssets(DuplicatiPhotosAssetMetadata *assets, size_t coun free(assets); } +// Maximum number of bytes that may be buffered in memory before the +// PhotoKit data-received handler is throttled. PhotoKit delivers data on a +// background thread as fast as it can (especially when downloading from +// iCloud), while the managed consumer drains it much more slowly during +// deduplication/compression/upload. Without a cap the entire asset (or many +// concurrent assets) accumulates in memory, which previously led to the +// process being killed by the OS out-of-memory reaper. 8 MiB is a generous +// window that keeps throughput high while bounding memory use. +static const NSUInteger kDuplicatiPhotosMaxBufferBytes = 8 * 1024 * 1024; + @interface DuplicatiPhotosReader : NSObject @property (nonatomic, strong) NSMutableData *buffer; +// Number of bytes at the front of `buffer` that have already been consumed. +// Tracking a read offset avoids repeatedly shifting the whole buffer (and +// avoids NSMutableData holding onto peak capacity) until we compact. +@property (nonatomic, assign) NSUInteger readOffset; @property (nonatomic, strong) NSCondition *condition; @property (nonatomic, assign) BOOL completed; @property (nonatomic, assign) BOOL cancelled; @@ -305,6 +319,7 @@ void DuplicatiPhotosFreeAssets(DuplicatiPhotosAssetMetadata *assets, size_t coun if (self) { _resource = resource; _buffer = [NSMutableData data]; + _readOffset = 0; _condition = [[NSCondition alloc] init]; _completed = NO; _cancelled = NO; @@ -329,9 +344,27 @@ void DuplicatiPhotosFreeAssets(DuplicatiPhotosAssetMetadata *assets, size_t coun DLog(@"DuplicatiPhotos: Received data chunk of size: %lu", (unsigned long)data.length); [strongSelf.condition lock]; - DLog(@"DuplicatiPhotos: Locked condition, current buffer size: %lu, appending data", (unsigned long)strongSelf.buffer.length); + + // Apply back-pressure: if the amount of buffered-but-unread + // data has grown past the cap, block this handler until the + // consumer has drained enough to make room. This throttles + // PhotoKit so it does not read the whole (potentially + // multi-gigabyte) asset into memory ahead of the consumer. + while (!strongSelf.cancelled && + (strongSelf.buffer.length - strongSelf.readOffset) >= kDuplicatiPhotosMaxBufferBytes) { + DLog(@"DuplicatiPhotos: Buffer full (%lu bytes), waiting for consumer to drain", (unsigned long)(strongSelf.buffer.length - strongSelf.readOffset)); + [strongSelf.condition wait]; + } + + if (strongSelf.cancelled) { + DLog(@"DuplicatiPhotos: Cancelled while waiting to append, dropping data"); + [strongSelf.condition unlock]; + return; + } + + DLog(@"DuplicatiPhotos: Locked condition, current unread size: %lu, appending data", (unsigned long)(strongSelf.buffer.length - strongSelf.readOffset)); [strongSelf.buffer appendData:data]; - DLog(@"DuplicatiPhotos: Data appended, new buffer size: %lu", (unsigned long)strongSelf.buffer.length); + DLog(@"DuplicatiPhotos: Data appended, new unread size: %lu", (unsigned long)(strongSelf.buffer.length - strongSelf.readOffset)); [strongSelf.condition signal]; DLog(@"DuplicatiPhotos: Signaled condition, unlocking"); [strongSelf.condition unlock]; @@ -402,14 +435,16 @@ void DuplicatiPhotosFreeAssets(DuplicatiPhotosAssetMetadata *assets, size_t coun DLog(@"DuplicatiPhotos: Locking condition for read on thread %@", [NSThread currentThread]); [self.condition lock]; - DLog(@"DuplicatiPhotos: Condition locked, buffer length: %lu, completed: %d", (unsigned long)self.buffer.length, self.completed); - while (self.buffer.length == 0 && !self.completed) { + NSUInteger available = self.buffer.length - self.readOffset; + DLog(@"DuplicatiPhotos: Condition locked, available: %lu, completed: %d", (unsigned long)available, self.completed); + while (available == 0 && !self.completed) { DLog(@"DuplicatiPhotos: Waiting for data..."); [self.condition wait]; - DLog(@"DuplicatiPhotos: Woke up from wait, buffer length: %lu, completed: %d", (unsigned long)self.buffer.length, self.completed); + available = self.buffer.length - self.readOffset; + DLog(@"DuplicatiPhotos: Woke up from wait, available: %lu, completed: %d", (unsigned long)available, self.completed); } - if (self.buffer.length == 0) { + if (available == 0) { DLog(@"DuplicatiPhotos: Buffer is empty after wait"); NSError *error = self.error; [self.condition unlock]; @@ -426,12 +461,30 @@ void DuplicatiPhotosFreeAssets(DuplicatiPhotosAssetMetadata *assets, size_t coun return 0; } - size_t toCopy = MIN((size_t)self.buffer.length, length); - DLog(@"DuplicatiPhotos: Copying %zu bytes to destination from buffer of size %lu", toCopy, (unsigned long)self.buffer.length); - memcpy(destination, self.buffer.bytes, toCopy); - DLog(@"DuplicatiPhotos: memcpy completed, removing copied bytes from buffer"); - [self.buffer replaceBytesInRange:NSMakeRange(0, toCopy) withBytes:NULL length:0]; - DLog(@"DuplicatiPhotos: Buffer updated, new size: %lu, unlocking", (unsigned long)self.buffer.length); + size_t toCopy = MIN((size_t)available, length); + DLog(@"DuplicatiPhotos: Copying %zu bytes to destination from available %lu", toCopy, (unsigned long)available); + memcpy(destination, (const uint8_t *)self.buffer.bytes + self.readOffset, toCopy); + self.readOffset += toCopy; + DLog(@"DuplicatiPhotos: memcpy completed, advancing read offset to %lu", (unsigned long)self.readOffset); + + // Reclaim memory. If everything buffered so far has been consumed, reset + // the buffer to zero length (releasing peak capacity). Otherwise, once the + // consumed prefix grows large, drop it from the front so the backing store + // does not grow without bound for large assets. Using a read offset avoids + // shifting bytes on every single read. + if (self.readOffset == self.buffer.length) { + self.buffer.length = 0; + self.readOffset = 0; + DLog(@"DuplicatiPhotos: Buffer fully drained, reset to empty"); + } else if (self.readOffset >= kDuplicatiPhotosMaxBufferBytes) { + [self.buffer replaceBytesInRange:NSMakeRange(0, self.readOffset) withBytes:NULL length:0]; + self.readOffset = 0; + DLog(@"DuplicatiPhotos: Compacted buffer, new size: %lu", (unsigned long)self.buffer.length); + } + + // Wake the (possibly throttled) data-received handler now that space is free. + [self.condition signal]; + DLog(@"DuplicatiPhotos: Signaled producer, unlocking, new unread size: %lu", (unsigned long)(self.buffer.length - self.readOffset)); [self.condition unlock]; DLog(@"DuplicatiPhotos: Returning %zd bytes", (ssize_t)toCopy); return (ssize_t)toCopy; diff --git a/Tools/MacOSPhotosNative/libDuplicatiPhotos.dylib b/Tools/MacOSPhotosNative/libDuplicatiPhotos.dylib index 998f992b8..5b8f19858 100755 Binary files a/Tools/MacOSPhotosNative/libDuplicatiPhotos.dylib and b/Tools/MacOSPhotosNative/libDuplicatiPhotos.dylib differ