From e6c98d364ff1633c12e8ee228771135e3955c765 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Fri, 16 Mar 2018 15:13:26 -0700 Subject: [PATCH] [NoCopyRendering] Use vm instead of malloc (#833) * [Contexts] Use mmap directly for possible perf gain and to tag the memory as CGImage * Wrap the mmap in an object * Go straight to dataprovider * Tweak it * Remove wrong comment * Finish that comment * Address warnings --- AsyncDisplayKit.xcodeproj/project.pbxproj | 10 ++- CHANGELOG.md | 7 +- Source/ASCGImageBuffer.h | 31 ++++++++ Source/ASCGImageBuffer.m | 92 +++++++++++++++++++++++ Source/Details/ASGraphicsContext.h | 2 +- Source/Details/ASGraphicsContext.m | 19 ++--- 6 files changed, 147 insertions(+), 14 deletions(-) create mode 100644 Source/ASCGImageBuffer.h create mode 100644 Source/ASCGImageBuffer.m diff --git a/AsyncDisplayKit.xcodeproj/project.pbxproj b/AsyncDisplayKit.xcodeproj/project.pbxproj index 06db270dcc..6cc4d105c1 100644 --- a/AsyncDisplayKit.xcodeproj/project.pbxproj +++ b/AsyncDisplayKit.xcodeproj/project.pbxproj @@ -357,6 +357,8 @@ CC6AA2DB1E9F03B900978E87 /* ASDisplayNode+Ancestry.m in Sources */ = {isa = PBXBuildFile; fileRef = CC6AA2D91E9F03B900978E87 /* ASDisplayNode+Ancestry.m */; }; CC7FD9E11BB5F750005CCB2B /* ASPhotosFrameworkImageRequestTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CC7FD9E01BB5F750005CCB2B /* ASPhotosFrameworkImageRequestTests.m */; }; CC7FD9E21BB603FF005CCB2B /* ASPhotosFrameworkImageRequest.h in Headers */ = {isa = PBXBuildFile; fileRef = CC7FD9DC1BB5E962005CCB2B /* ASPhotosFrameworkImageRequest.h */; settings = {ATTRIBUTES = (Public, ); }; }; + CC84C7F220474C5300A3851B /* ASCGImageBuffer.h in Headers */ = {isa = PBXBuildFile; fileRef = CC84C7F020474C5300A3851B /* ASCGImageBuffer.h */; }; + CC84C7F320474C5300A3851B /* ASCGImageBuffer.m in Sources */ = {isa = PBXBuildFile; fileRef = CC84C7F120474C5300A3851B /* ASCGImageBuffer.m */; }; CC87BB951DA8193C0090E380 /* ASCellNode+Internal.h in Headers */ = {isa = PBXBuildFile; fileRef = CC87BB941DA8193C0090E380 /* ASCellNode+Internal.h */; settings = {ATTRIBUTES = (Private, ); }; }; CC8B05D61D73836400F54286 /* ASPerformanceTestContext.m in Sources */ = {isa = PBXBuildFile; fileRef = CC8B05D51D73836400F54286 /* ASPerformanceTestContext.m */; }; CC8B05D81D73979700F54286 /* ASTextNodePerformanceTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CC8B05D71D73979700F54286 /* ASTextNodePerformanceTests.m */; }; @@ -853,6 +855,8 @@ CC7FD9DC1BB5E962005CCB2B /* ASPhotosFrameworkImageRequest.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASPhotosFrameworkImageRequest.h; sourceTree = ""; }; CC7FD9DD1BB5E962005CCB2B /* ASPhotosFrameworkImageRequest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASPhotosFrameworkImageRequest.m; sourceTree = ""; }; CC7FD9E01BB5F750005CCB2B /* ASPhotosFrameworkImageRequestTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASPhotosFrameworkImageRequestTests.m; sourceTree = ""; }; + CC84C7F020474C5300A3851B /* ASCGImageBuffer.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ASCGImageBuffer.h; sourceTree = ""; }; + CC84C7F120474C5300A3851B /* ASCGImageBuffer.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ASCGImageBuffer.m; sourceTree = ""; }; CC87BB941DA8193C0090E380 /* ASCellNode+Internal.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "ASCellNode+Internal.h"; sourceTree = ""; }; CC8B05D41D73836400F54286 /* ASPerformanceTestContext.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASPerformanceTestContext.h; sourceTree = ""; }; CC8B05D51D73836400F54286 /* ASPerformanceTestContext.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASPerformanceTestContext.m; sourceTree = ""; }; @@ -1078,6 +1082,8 @@ 058D09B1195D04C000B7D73C /* Source */ = { isa = PBXGroup; children = ( + CC84C7F020474C5300A3851B /* ASCGImageBuffer.h */, + CC84C7F120474C5300A3851B /* ASCGImageBuffer.m */, CCED5E3C2020D36800395C40 /* ASNetworkImageLoadInfo.h */, CCED5E3D2020D36800395C40 /* ASNetworkImageLoadInfo.m */, CCE04B1D1E313E99006AEBBB /* Collection Data Adapter */, @@ -1900,6 +1906,7 @@ DEC146B71C37A16A004A0EE7 /* ASCollectionInternal.h in Headers */, 68B8A4E21CBDB958007E4543 /* ASWeakProxy.h in Headers */, 9F98C0271DBE29FC00476D92 /* ASControlTargetAction.h in Headers */, + CC84C7F220474C5300A3851B /* ASCGImageBuffer.h in Headers */, 695943401D70815300B0EE1F /* ASDisplayNodeLayout.h in Headers */, 0442850E1BAA64EC00D16268 /* ASTwoDimensionalArrayUtils.h in Headers */, DE8BEAC21C2DF3FC00D57C12 /* ASDelegateProxy.h in Headers */, @@ -2045,7 +2052,7 @@ attributes = { CLASSPREFIX = AS; LastUpgradeCheck = 0820; - ORGANIZATIONNAME = Facebook; + ORGANIZATIONNAME = Pinterest; TargetAttributes = { 057D02BE1AC0A66700C7AC3C = { CreatedOnToolsVersion = 6.2; @@ -2411,6 +2418,7 @@ 697796611D8AC8D3007E93D7 /* ASLayoutSpec+Subclasses.mm in Sources */, B350623B1B010EFD0018CF92 /* NSMutableAttributedString+TextKitAdditions.m in Sources */, CCA282CD1E9EB73E0037E8B7 /* ASTipNode.m in Sources */, + CC84C7F320474C5300A3851B /* ASCGImageBuffer.m in Sources */, 044284FD1BAA365100D16268 /* UICollectionViewLayout+ASConvenience.m in Sources */, CC0F885B1E42807F00576FED /* ASCollectionViewFlowLayoutInspector.m in Sources */, 690ED5981E36D118000627C0 /* ASControlNode+tvOS.m in Sources */, diff --git a/CHANGELOG.md b/CHANGELOG.md index e27918bad5..17d91da247 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,10 +32,11 @@ - Make `ASCellNode` tint color apply to table view cell accessories. [Vladyslav Chapaev](https://github.com/ShogunPhyched) [#764](https://github.com/TextureGroup/Texture/pull/764) - Fix ASTextNode2 is accessing backgroundColor off main while sizing / layout is happening. [Michael Schneider](https://github.com/maicki) [#794](https://github.com/TextureGroup/Texture/pull/778/) - Pass scrollViewWillEndDragging delegation through in ASIGListAdapterDataSource for IGListKit integration. [#796](https://github.com/TextureGroup/Texture/pull/796) -- Fix UIResponder handling with view backing ASDisplayNode. [Michael Schneider](https://github.com/maicki) [#789] (https://github.com/TextureGroup/Texture/pull/789/) -- Optimized thread-local storage by replacing pthread_specific with C11 thread-local variables. [Adlai Holler](https://github.com/Adlai-Holler) [#811] (https://github.com/TextureGroup/Texture/pull/811/) -- Fixed a thread-sanitizer warning in ASTextNode. [Adlai Holler](https://github.com/Adlai-Holler) [#830] (https://github.com/TextureGroup/Texture/pull/830/) +- Fix UIResponder handling with view backing ASDisplayNode. [Michael Schneider](https://github.com/maicki) [#789](https://github.com/TextureGroup/Texture/pull/789/) +- Optimized thread-local storage by replacing pthread_specific with C11 thread-local variables. [Adlai Holler](https://github.com/Adlai-Holler) [#811](https://github.com/TextureGroup/Texture/pull/811/) +- Fixed a thread-sanitizer warning in ASTextNode. [Adlai Holler](https://github.com/Adlai-Holler) [#830](https://github.com/TextureGroup/Texture/pull/830/) - Fix ASTextNode2 handling background color incorrectly. [Adlai Holler](https://github.com/Adlai-Holler) [#831] (https://github.com/TextureGroup/Texture/pull/831/) +- [NoCopyRendering] Improved performance & fixed image memory not being tagged in Instruments. [Adlai Holler](https://github.com/Adlai-Holler) [#833](https://github.com/TextureGroup/Texture/pull/833/) ## 2.6 - [Xcode 9] Updated to require Xcode 9 (to fix warnings) [Garrett Moon](https://github.com/garrettmoon) diff --git a/Source/ASCGImageBuffer.h b/Source/ASCGImageBuffer.h new file mode 100644 index 0000000000..47d4b8d306 --- /dev/null +++ b/Source/ASCGImageBuffer.h @@ -0,0 +1,31 @@ +// +// ASCGImageBuffer.h +// Texture +// +// Copyright (c) 2018-present, Pinterest, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// + +#import +#import +#import + +NS_ASSUME_NONNULL_BEGIN + +AS_SUBCLASSING_RESTRICTED +@interface ASCGImageBuffer : NSObject + +- (instancetype)initWithLength:(NSUInteger)length; + +@property (readonly) void *mutableBytes NS_RETURNS_INNER_POINTER; + +/// Don't do any drawing or call any methods after calling this. +- (CGDataProviderRef)createDataProviderAndInvalidate; + +@end + +NS_ASSUME_NONNULL_END diff --git a/Source/ASCGImageBuffer.m b/Source/ASCGImageBuffer.m new file mode 100644 index 0000000000..f1b9d961a7 --- /dev/null +++ b/Source/ASCGImageBuffer.m @@ -0,0 +1,92 @@ +// +// ASCGImageBuffer.m +// Texture +// +// Copyright (c) 2018-present, Pinterest, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// + +#import "ASCGImageBuffer.h" + +#import +#import +#import +#import + +/** + * The behavior of this class is modeled on the private function + * _CGDataProviderCreateWithCopyOfData, which is the function used + * by CGBitmapContextCreateImage. + * + * If the buffer is larger than a page, we use mmap and mark it as + * read-only when they are finished drawing. Then we wrap the VM + * in an NSData + */ +@implementation ASCGImageBuffer { + BOOL _createdData; + BOOL _isVM; + NSUInteger _length; +} + +- (instancetype)initWithLength:(NSUInteger)length +{ + if (self = [super init]) { + _length = length; + _isVM = NO;//(length >= vm_page_size); + if (_isVM) { + _mutableBytes = mmap(NULL, length, PROT_WRITE | PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, VM_MAKE_TAG(VM_MEMORY_COREGRAPHICS_DATA), 0); + if (_mutableBytes == MAP_FAILED) { + NSAssert(NO, @"Failed to map for CG image data."); + _isVM = NO; + } + } + + // Check the VM flag again because we may have failed above. + if (!_isVM) { + _mutableBytes = malloc(length); + } + } + return self; +} + +- (void)dealloc +{ + if (!_createdData) { + [ASCGImageBuffer deallocateBuffer:_mutableBytes length:_length isVM:_isVM]; + } +} + +- (CGDataProviderRef)createDataProviderAndInvalidate +{ + NSAssert(!_createdData, @"Should not create data provider from buffer multiple times."); + _createdData = YES; + + // Mark the pages as read-only. + if (_isVM) { + __unused kern_return_t result = vm_protect(mach_task_self(), (vm_address_t)_mutableBytes, _length, true, VM_PROT_READ); + NSAssert(result == noErr, @"Error marking buffer as read-only: %@", [NSError errorWithDomain:NSMachErrorDomain code:result userInfo:nil]); + } + + // Wrap in an NSData + BOOL isVM = _isVM; + NSData *d = [[NSData alloc] initWithBytesNoCopy:_mutableBytes length:_length deallocator:^(void * _Nonnull bytes, NSUInteger length) { + [ASCGImageBuffer deallocateBuffer:bytes length:length isVM:isVM]; + }]; + return CGDataProviderCreateWithCFData((__bridge CFDataRef)d); +} + ++ (void)deallocateBuffer:(void *)buf length:(NSUInteger)length isVM:(BOOL)isVM +{ + if (isVM) { + __unused kern_return_t result = vm_deallocate(mach_task_self(), (vm_address_t)buf, length); + NSAssert(result == noErr, @"Failed to unmap cg image buffer: %@", [NSError errorWithDomain:NSMachErrorDomain code:result userInfo:nil]); + } else { + free(buf); + } +} + +@end diff --git a/Source/Details/ASGraphicsContext.h b/Source/Details/ASGraphicsContext.h index 26183d5951..b7f808c970 100644 --- a/Source/Details/ASGraphicsContext.h +++ b/Source/Details/ASGraphicsContext.h @@ -52,7 +52,7 @@ extern void ASGraphicsBeginImageContextWithOptions(CGSize size, BOOL opaque, CGF * * Behavior is the same as UIGraphicsGetImageFromCurrentImageContext followed by UIGraphicsEndImageContext. */ -extern UIImage * _Nullable ASGraphicsGetImageAndEndCurrentContext(void); +extern UIImage * _Nullable ASGraphicsGetImageAndEndCurrentContext(void) NS_RETURNS_RETAINED; /** * Call this if you want to end the current context without making an image. diff --git a/Source/Details/ASGraphicsContext.m b/Source/Details/ASGraphicsContext.m index 3aaa14c090..70294d10ae 100644 --- a/Source/Details/ASGraphicsContext.m +++ b/Source/Details/ASGraphicsContext.m @@ -11,6 +11,7 @@ // #import "ASGraphicsContext.h" +#import #import #import #import @@ -105,12 +106,13 @@ extern void ASGraphicsBeginImageContextWithOptions(CGSize size, BOOL opaque, CGF // We create our own buffer, and wrap the context around that. This way we can prevent // the copy that usually gets made when you form a CGImage from the context. - NSMutableData *data = [[NSMutableData alloc] initWithLength:bufferSize]; - CGContextRef context = CGBitmapContextCreate(data.mutableBytes, intWidth, intHeight, bitsPerComponent, bytesPerRow, colorspace, bitmapInfo); + ASCGImageBuffer *buffer = [[ASCGImageBuffer alloc] initWithLength:bufferSize]; + + CGContextRef context = CGBitmapContextCreate(buffer.mutableBytes, intWidth, intHeight, bitsPerComponent, bytesPerRow, colorspace, bitmapInfo); // Transfer ownership of the data to the context. So that if the context // is destroyed before we create an image from it, the data will be released. - objc_setAssociatedObject((__bridge id)context, &__contextDataAssociationKey, data, OBJC_ASSOCIATION_RETAIN_NONATOMIC); + objc_setAssociatedObject((__bridge id)context, &__contextDataAssociationKey, buffer, OBJC_ASSOCIATION_RETAIN_NONATOMIC); // Set the CTM to account for iOS orientation & specified scale. // If only we could use CGContextSetBaseCTM. It doesn't @@ -128,7 +130,7 @@ extern void ASGraphicsBeginImageContextWithOptions(CGSize size, BOOL opaque, CGF CGContextRelease(context); } -extern UIImage * _Nullable ASGraphicsGetImageAndEndCurrentContext() +extern UIImage * _Nullable ASGraphicsGetImageAndEndCurrentContext() NS_RETURNS_RETAINED { if (!ASNoCopyRenderingBlockAndCheckEnabled()) { UIImage *image = UIGraphicsGetImageFromCurrentImageContext(); @@ -159,11 +161,10 @@ extern UIImage * _Nullable ASGraphicsGetImageAndEndCurrentContext() UIGraphicsEndImageContext(); }); - // Retrieve our data and wrap it in a CGDataProvider. - // Don't worry, the provider doesn't copy the data – it just retains it. - NSMutableData *data = objc_getAssociatedObject((__bridge id)context, &__contextDataAssociationKey); - ASDisplayNodeCAssertNotNil(data, nil); - CGDataProviderRef provider = CGDataProviderCreateWithCFData((__bridge CFDataRef)data); + // Retrieve our buffer and create a CGDataProvider from it. + ASCGImageBuffer *buffer = objc_getAssociatedObject((__bridge id)context, &__contextDataAssociationKey); + ASDisplayNodeCAssertNotNil(buffer, nil); + CGDataProviderRef provider = [buffer createDataProviderAndInvalidate]; // Create the CGImage. Options taken from CGBitmapContextCreateImage. CGImageRef cgImg = CGImageCreate(CGBitmapContextGetWidth(context), CGBitmapContextGetHeight(context), CGBitmapContextGetBitsPerComponent(context), CGBitmapContextGetBitsPerPixel(context), CGBitmapContextGetBytesPerRow(context), imageColorSpace, CGBitmapContextGetBitmapInfo(context), provider, NULL, true, kCGRenderingIntentDefault);