Skip to content

Revert "Eagerly move attachable values to the heap. (#809)" #810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 6, 2024
Merged

Revert "Eagerly move attachable values to the heap. (#809)" #810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 4 additions & 69 deletions Sources/Testing/Attachments/Test.Attachment.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,7 @@ extension Test {
named preferredName: String? = nil,
sourceLocation: SourceLocation = #_sourceLocation
) {
if _shouldMoveValueToHeap(attachableValue) {
self.attachableValue = _HeapAllocatedAttachableProxy(rawValue: attachableValue)
} else {
self.attachableValue = attachableValue
}
self.attachableValue = attachableValue
self.preferredName = preferredName ?? Self.defaultPreferredName
self.sourceLocation = sourceLocation
}
Expand All @@ -99,71 +95,10 @@ extension Test.Attachment {
}
}

// MARK: - Heap allocation optimizations

/// A type that stands in for an attachable value of value type (as opposed to
/// reference type.)
///
/// We don't know the in-memory layout of an attachable value; it may be very
/// expensive to make the typical copies Swift makes over the course of a test
/// run if a value is large or contains lots of references to objects. So we
/// eagerly move attachments of value type to the heap by ensconcing them in
/// instances of this type.
private final class _HeapAllocatedAttachableProxy<RawValue>: RawRepresentable, Test.Attachable, Sendable where RawValue: Test.Attachable & Sendable {
let rawValue: RawValue

init(rawValue: RawValue) {
self.rawValue = rawValue
}

var estimatedAttachmentByteCount: Int? {
rawValue.estimatedAttachmentByteCount
}

func withUnsafeBufferPointer<R>(for attachment: borrowing Test.Attachment, _ body: (UnsafeRawBufferPointer) throws -> R) throws -> R {
try rawValue.withUnsafeBufferPointer(for: attachment, body)
}
}

/// Whether or not an attachable value should be moved to the heap when adding
/// it to an attachment.
///
/// - Parameters:
/// - attachableValue: The value that is being attached.
///
/// - Returns: Whether or not `attachableValue` should be boxed in an instance
/// of `_HeapAllocatedAttachableProxy`.
private func _shouldMoveValueToHeap<T>(_ attachableValue: borrowing T) -> Bool where T: Test.Attachable {
// Do not (redundantly) move existing heap-allocated objects to the heap.
if T.self is AnyClass {
return false
}

// Do not move small POD value types to the heap.
//
// _isPOD() is defined in the standard library and is approximately equivalent
// to testing for conformance to BitwiseCopyable, but it's a marker protocol
// which means we can't test for conformance in a generic context.
//
// The size limit used is the size of an existential box's inline storage (see
// NumWords_ValueBuffer in the Swift repository.) Such values can be directly
// stored inside an `Attachment` and are cheap to copy.
if _isPOD(T.self) && MemoryLayout<T>.stride <= (MemoryLayout<Int>.stride * 3) {
return false
}

return true
}

// MARK: - Non-sendable and move-only attachments

/// A type that stands in for an attachable value that is not also sendable.
///
/// This type is a value type so that we can mutate its properties while
/// transferring information from the original attachable value. Consequently,
/// instances will be boxed in an instance of `_HeapAllocatedAttachableProxy`
/// when they are used to initialize instances of ``Test/Attachment``.
private struct _EagerlyCopiedAttachableProxy: Test.Attachable, Sendable {
/// A type that stands in for an attachable type that is not also sendable.
private struct _AttachableProxy: Test.Attachable, Sendable {
/// The result of `withUnsafeBufferPointer(for:_:)` from the original
/// attachable value.
var encodedValue = [UInt8]()
Expand Down Expand Up @@ -200,7 +135,7 @@ extension Test.Attachment {
named preferredName: String? = nil,
sourceLocation: SourceLocation = #_sourceLocation
) {
var proxyAttachable = _EagerlyCopiedAttachableProxy()
var proxyAttachable = _AttachableProxy()
proxyAttachable.estimatedAttachmentByteCount = attachableValue.estimatedAttachmentByteCount

// BUG: the borrow checker thinks that withErrorRecording() is consuming
Expand Down
90 changes: 6 additions & 84 deletions Tests/TestingTests/AttachmentTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,58 +199,6 @@ struct AttachmentTests {
}
}

@Test func attachRefCountedValue() throws {
let attachableValue = MySendableAttachableWithRefcountedSemantics(values: (1, 2, 3))
let attachment = Test.Attachment(attachableValue, named: "loremipsum")

#expect(attachment.attachableValue is MySendableAttachableWithRefcountedSemantics)
#expect(attachment.attachableValue.estimatedAttachmentByteCount == attachableValue.estimatedAttachmentByteCount)
try attachment.attachableValue.withUnsafeBufferPointer(for: attachment) { buffer in
withUnsafeBytes(of: (1, 2, 3)) { buffer2 in
#expect(buffer.elementsEqual(buffer2))
}
}
}

@Test func attachSmallPODValue() throws {
let attachableValue = MySendableAttachableWithSmallPODSemantics(values: (1, 2, 3))
let attachment = Test.Attachment(attachableValue, named: "loremipsum")

#expect(!(type(of: attachment.attachableValue) is AnyClass))
#expect(attachment.attachableValue.estimatedAttachmentByteCount == attachableValue.estimatedAttachmentByteCount)
try attachment.attachableValue.withUnsafeBufferPointer(for: attachment) { buffer in
withUnsafeBytes(of: (1, 2, 3)) { buffer2 in
#expect(buffer.elementsEqual(buffer2))
}
}
}

@Test func attachBigPODValue() throws {
let attachableValue = MySendableAttachableWithBigPODSemantics(values: (1, 2, 3, 4, 5, 6))
let attachment = Test.Attachment(attachableValue, named: "loremipsum")

#expect(type(of: attachment.attachableValue) is AnyClass)
#expect(attachment.attachableValue.estimatedAttachmentByteCount == attachableValue.estimatedAttachmentByteCount)
try attachment.attachableValue.withUnsafeBufferPointer(for: attachment) { buffer in
withUnsafeBytes(of: (1, 2, 3, 4, 5, 6)) { buffer2 in
#expect(buffer.elementsEqual(buffer2))
}
}
}

@Test func attachNonPODValue() throws {
let attachableValue = MySendableAttachableWithNonPODSemantics(values: (1, 2, 3))
let attachment = Test.Attachment(attachableValue, named: "loremipsum")

#expect(type(of: attachment.attachableValue) is AnyClass)
#expect(attachment.attachableValue.estimatedAttachmentByteCount == attachableValue.estimatedAttachmentByteCount)
try attachment.attachableValue.withUnsafeBufferPointer(for: attachment) { buffer in
withUnsafeBytes(of: (1, 2, 3)) { buffer2 in
#expect(buffer.elementsEqual(buffer2))
}
}
}

@Test func issueRecordedWhenAttachingNonSendableValueThatThrows() async {
await confirmation("Attachment detected") { valueAttached in
await confirmation("Issue recorded") { issueRecorded in
Expand Down Expand Up @@ -374,39 +322,13 @@ struct MySendableAttachable: Test.Attachable, Sendable {
}
}

final class MySendableAttachableWithRefcountedSemantics: Test.Attachable, Sendable {
let values: (Int, Int, Int)

init(values: (Int, Int, Int)) {
self.values = values
}

func withUnsafeBufferPointer<R>(for attachment: borrowing Testing.Test.Attachment, _ body: (UnsafeRawBufferPointer) throws -> R) throws -> R {
try withUnsafeBytes(of: values, body)
}
}

struct MySendableAttachableWithSmallPODSemantics: Test.Attachable, Sendable {
var values: (Int, Int, Int)

func withUnsafeBufferPointer<R>(for attachment: borrowing Testing.Test.Attachment, _ body: (UnsafeRawBufferPointer) throws -> R) throws -> R {
try withUnsafeBytes(of: values, body)
}
}

struct MySendableAttachableWithBigPODSemantics: Test.Attachable, Sendable {
var values: (Int, Int, Int, Int, Int, Int)

func withUnsafeBufferPointer<R>(for attachment: borrowing Testing.Test.Attachment, _ body: (UnsafeRawBufferPointer) throws -> R) throws -> R {
try withUnsafeBytes(of: values, body)
}
}

struct MySendableAttachableWithNonPODSemantics: Test.Attachable, Sendable {
var values: (Int, Int, Int)
var makeItComplicated = "it's complicated"
struct MySendableAttachableWithDefaultByteCount: Test.Attachable, Sendable {
var string: String

func withUnsafeBufferPointer<R>(for attachment: borrowing Testing.Test.Attachment, _ body: (UnsafeRawBufferPointer) throws -> R) throws -> R {
try withUnsafeBytes(of: values, body)
var string = string
return try string.withUTF8 { buffer in
try body(.init(buffer))
}
}
}