From 8bd5fdac1727a2d30933e9da08b8250f26d748a8 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Fri, 17 Jan 2025 15:30:56 -0500 Subject: [PATCH] Don't allocate a buffer on the heap when enumerating type metadata. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR eliminates the need for us to allocate a large buffer to contain type metadata pointers we discover in type metadata sections using the legacy test discovery mechanism. Why do I keep opening these PRs? Because for each line of C++ I remove from the Swift toolchain, I get a cookie! 🍪 --- Sources/Testing/Test+Discovery+Legacy.swift | 10 +-- Sources/_TestingInternals/Discovery.cpp | 64 +++++++------------ Sources/_TestingInternals/include/Discovery.h | 29 ++++----- 3 files changed, 40 insertions(+), 63 deletions(-) diff --git a/Sources/Testing/Test+Discovery+Legacy.swift b/Sources/Testing/Test+Discovery+Legacy.swift index c90e8c590..6ee080051 100644 --- a/Sources/Testing/Test+Discovery+Legacy.swift +++ b/Sources/Testing/Test+Discovery+Legacy.swift @@ -55,12 +55,8 @@ let exitTestContainerTypeNameMagic = "__🟠$exit_test_body__" /// - Returns: A sequence of Swift types whose names contain `nameSubstring`. func types(withNamesContaining nameSubstring: String) -> some Sequence { SectionBounds.all(.typeMetadata).lazy.flatMap { sb in - var count = 0 - let start = swt_copyTypes(in: sb.buffer.baseAddress!, sb.buffer.count, withNamesContaining: nameSubstring, count: &count) - defer { - free(start) - } - return UnsafeBufferPointer(start: start, count: count) - .withMemoryRebound(to: Any.Type.self) { Array($0) } + stride(from: sb.buffer.baseAddress!, to: sb.buffer.baseAddress! + sb.buffer.count, by: SWTTypeMetadataRecordByteCount).lazy + .compactMap { swt_getType(fromTypeMetadataRecord: $0, ifNameContains: nameSubstring) } + .map { unsafeBitCast($0, to: Any.Type.self) } } } diff --git a/Sources/_TestingInternals/Discovery.cpp b/Sources/_TestingInternals/Discovery.cpp index f66eaf874..6173126c1 100644 --- a/Sources/_TestingInternals/Discovery.cpp +++ b/Sources/_TestingInternals/Discovery.cpp @@ -10,11 +10,9 @@ #include "Discovery.h" -#include #include #include #include -#include #if defined(SWT_NO_DYNAMIC_LINKING) #pragma mark - Statically-linked section bounds @@ -189,46 +187,32 @@ struct SWTTypeMetadataRecord { #pragma mark - Legacy test discovery -void **swt_copyTypesWithNamesContaining(const void *sectionBegin, size_t sectionSize, const char *nameSubstring, size_t *outCount) { - void **result = nullptr; - size_t resultCount = 0; - - auto records = reinterpret_cast(sectionBegin); - size_t recordCount = sectionSize / sizeof(SWTTypeMetadataRecord); - for (size_t i = 0; i < recordCount; i++) { - auto contextDescriptor = records[i].getContextDescriptor(); - if (!contextDescriptor) { - // This type metadata record is invalid (or we don't understand how to - // get its context descriptor), so skip it. - continue; - } else if (contextDescriptor->isGeneric()) { - // Generic types cannot be fully instantiated without generic - // parameters, which is not something we can know abstractly. - continue; - } +const size_t SWTTypeMetadataRecordByteCount = sizeof(SWTTypeMetadataRecord); - // Check that the type's name passes. This will be more expensive than the - // checks above, but should be cheaper than realizing the metadata. - const char *typeName = contextDescriptor->getName(); - bool nameOK = typeName && nullptr != std::strstr(typeName, nameSubstring); - if (!nameOK) { - continue; - } +const void *swt_getTypeFromTypeMetadataRecord(const void *recordAddress, const char *nameSubstring) { + auto record = reinterpret_cast(recordAddress); + auto contextDescriptor = record->getContextDescriptor(); + if (!contextDescriptor) { + // This type metadata record is invalid (or we don't understand how to + // get its context descriptor), so skip it. + return nullptr; + } else if (contextDescriptor->isGeneric()) { + // Generic types cannot be fully instantiated without generic + // parameters, which is not something we can know abstractly. + return nullptr; + } - if (void *typeMetadata = contextDescriptor->getMetadata()) { - if (!result) { - // This is the first matching type we've found. That presumably means - // we'll find more, so allocate enough space for all remaining types in - // the section. Is this necessarily space-efficient? No, but this - // allocation is short-lived and is immediately copied and freed in the - // Swift caller. - result = reinterpret_cast(std::calloc(recordCount - i, sizeof(void *))); - } - result[resultCount] = typeMetadata; - resultCount += 1; - } + // Check that the type's name passes. This will be more expensive than the + // checks above, but should be cheaper than realizing the metadata. + const char *typeName = contextDescriptor->getName(); + bool nameOK = typeName && nullptr != std::strstr(typeName, nameSubstring); + if (!nameOK) { + return nullptr; + } + + if (void *typeMetadata = contextDescriptor->getMetadata()) { + return typeMetadata; } - *outCount = resultCount; - return result; + return nullptr; } diff --git a/Sources/_TestingInternals/include/Discovery.h b/Sources/_TestingInternals/include/Discovery.h index cbcfc2d79..9d84b9f8c 100644 --- a/Sources/_TestingInternals/include/Discovery.h +++ b/Sources/_TestingInternals/include/Discovery.h @@ -84,25 +84,22 @@ SWT_EXTERN const void *_Nonnull const SWTTypeMetadataSectionBounds[2]; #pragma mark - Legacy test discovery -/// Copy all types known to Swift found in the given type metadata section with -/// a name containing the given substring. +/// The size, in bytes, of a Swift type metadata record. +SWT_EXTERN const size_t SWTTypeMetadataRecordByteCount; + +/// Get the type represented by the type metadata record at the given address if +/// its name contains the given string. /// /// - Parameters: -/// - sectionBegin: The address of the first byte of the Swift type metadata -/// section. -/// - sectionSize: The size, in bytes, of the Swift type metadata section. -/// - nameSubstring: A string which the names of matching classes all contain. -/// - outCount: On return, the number of type metadata pointers returned. +/// - recordAddress: The address of the Swift type metadata record. +/// - nameSubstring: A string which the names of matching types contain. /// -/// - Returns: A pointer to an array of type metadata pointers, or `nil` if no -/// matching types were found. The caller is responsible for freeing this -/// memory with `free()` when done. -SWT_EXTERN void *_Nonnull *_Nullable swt_copyTypesWithNamesContaining( - const void *sectionBegin, - size_t sectionSize, - const char *nameSubstring, - size_t *outCount -) SWT_SWIFT_NAME(swt_copyTypes(in:_:withNamesContaining:count:)); +/// - Returns: A Swift metatype (as `const void *`) or `nullptr` if it wasn't a +/// usable type metadata record or its name did not contain `nameSubstring`. +SWT_EXTERN const void *_Nullable swt_getTypeFromTypeMetadataRecord( + const void *recordAddress, + const char *nameSubstring +) SWT_SWIFT_NAME(swt_getType(fromTypeMetadataRecord:ifNameContains:)); SWT_ASSUME_NONNULL_END