Skip to content

Commit 3a537b8

Browse files
authored
Eagerly move attachable values to the heap. (#809)
This PR modifies `Test.Attachment.init(...)` so that, if the attachable value passed to it is of a large or non-bitwise-copyable value type, we store it in a heap-allocated container object. Why do we want to do this? Attachable values have arbitrary types we don't really know anything about, and may be very expensive to copy if they are large or if they contain many references to other objects. Since attachments are sendable and need to pass through many layers in the system, the cost of making copies for these attachable values can add up quickly. So, instead of copying them repeatedly, if we store them in a refcounted container and pass _that_ value around, we only infer a single copy cost. There are four modes for attachable value storage after this PR: ```mermaid flowchart TB; S@{ shape: diam, label: "Sendable?" }; C@{ shape: diam, label: "Copyable?" }; AO@{ shape: diam, label: "AnyObject?" }; BC@{ shape: diam, label: "BitwiseCopyable (POD)?" }; SMOL@{ shape: diam, label: "Fits in an existential box?" }; MO@{ shape: das, label: "In a heap-allocated box\n(eagerly serialized)" }; EX@{ shape: das, label: "Inline in an existential box" }; H@{ shape: das, label: "In a heap-allocated box" }; RC@{ shape: das, label: "Directly on the heap" }; S-- Yes ---C C-- Yes ---AO AO-- Yes ---RC AO-- No ---BC BC-- Yes ---SMOL BC-- No ---H SMOL-- Yes ---EX SMOL-- No ---H C-- No ---MO S-- No ---MO ``` ### Checklist: - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
1 parent 6a2b04c commit 3a537b8

File tree

2 files changed

+153
-10
lines changed

2 files changed

+153
-10
lines changed

Sources/Testing/Attachments/Test.Attachment.swift

Lines changed: 69 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,11 @@ extension Test {
6868
named preferredName: String? = nil,
6969
sourceLocation: SourceLocation = #_sourceLocation
7070
) {
71-
self.attachableValue = attachableValue
71+
if _shouldMoveValueToHeap(attachableValue) {
72+
self.attachableValue = _HeapAllocatedAttachableProxy(rawValue: attachableValue)
73+
} else {
74+
self.attachableValue = attachableValue
75+
}
7276
self.preferredName = preferredName ?? Self.defaultPreferredName
7377
self.sourceLocation = sourceLocation
7478
}
@@ -95,10 +99,71 @@ extension Test.Attachment {
9599
}
96100
}
97101

102+
// MARK: - Heap allocation optimizations
103+
104+
/// A type that stands in for an attachable value of value type (as opposed to
105+
/// reference type.)
106+
///
107+
/// We don't know the in-memory layout of an attachable value; it may be very
108+
/// expensive to make the typical copies Swift makes over the course of a test
109+
/// run if a value is large or contains lots of references to objects. So we
110+
/// eagerly move attachments of value type to the heap by ensconcing them in
111+
/// instances of this type.
112+
private final class _HeapAllocatedAttachableProxy<RawValue>: RawRepresentable, Test.Attachable, Sendable where RawValue: Test.Attachable & Sendable {
113+
let rawValue: RawValue
114+
115+
init(rawValue: RawValue) {
116+
self.rawValue = rawValue
117+
}
118+
119+
var estimatedAttachmentByteCount: Int? {
120+
rawValue.estimatedAttachmentByteCount
121+
}
122+
123+
func withUnsafeBufferPointer<R>(for attachment: borrowing Test.Attachment, _ body: (UnsafeRawBufferPointer) throws -> R) throws -> R {
124+
try rawValue.withUnsafeBufferPointer(for: attachment, body)
125+
}
126+
}
127+
128+
/// Whether or not an attachable value should be moved to the heap when adding
129+
/// it to an attachment.
130+
///
131+
/// - Parameters:
132+
/// - attachableValue: The value that is being attached.
133+
///
134+
/// - Returns: Whether or not `attachableValue` should be boxed in an instance
135+
/// of `_HeapAllocatedAttachableProxy`.
136+
private func _shouldMoveValueToHeap<T>(_ attachableValue: borrowing T) -> Bool where T: Test.Attachable {
137+
// Do not (redundantly) move existing heap-allocated objects to the heap.
138+
if T.self is AnyClass {
139+
return false
140+
}
141+
142+
// Do not move small POD value types to the heap.
143+
//
144+
// _isPOD() is defined in the standard library and is approximately equivalent
145+
// to testing for conformance to BitwiseCopyable, but it's a marker protocol
146+
// which means we can't test for conformance in a generic context.
147+
//
148+
// The size limit used is the size of an existential box's inline storage (see
149+
// NumWords_ValueBuffer in the Swift repository.) Such values can be directly
150+
// stored inside an `Attachment` and are cheap to copy.
151+
if _isPOD(T.self) && MemoryLayout<T>.stride <= (MemoryLayout<Int>.stride * 3) {
152+
return false
153+
}
154+
155+
return true
156+
}
157+
98158
// MARK: - Non-sendable and move-only attachments
99159

100-
/// A type that stands in for an attachable type that is not also sendable.
101-
private struct _AttachableProxy: Test.Attachable, Sendable {
160+
/// A type that stands in for an attachable value that is not also sendable.
161+
///
162+
/// This type is a value type so that we can mutate its properties while
163+
/// transferring information from the original attachable value. Consequently,
164+
/// instances will be boxed in an instance of `_HeapAllocatedAttachableProxy`
165+
/// when they are used to initialize instances of ``Test/Attachment``.
166+
private struct _EagerlyCopiedAttachableProxy: Test.Attachable, Sendable {
102167
/// The result of `withUnsafeBufferPointer(for:_:)` from the original
103168
/// attachable value.
104169
var encodedValue = [UInt8]()
@@ -135,7 +200,7 @@ extension Test.Attachment {
135200
named preferredName: String? = nil,
136201
sourceLocation: SourceLocation = #_sourceLocation
137202
) {
138-
var proxyAttachable = _AttachableProxy()
203+
var proxyAttachable = _EagerlyCopiedAttachableProxy()
139204
proxyAttachable.estimatedAttachmentByteCount = attachableValue.estimatedAttachmentByteCount
140205

141206
// BUG: the borrow checker thinks that withErrorRecording() is consuming

Tests/TestingTests/AttachmentTests.swift

Lines changed: 84 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,58 @@ struct AttachmentTests {
199199
}
200200
}
201201

202+
@Test func attachRefCountedValue() throws {
203+
let attachableValue = MySendableAttachableWithRefcountedSemantics(values: (1, 2, 3))
204+
let attachment = Test.Attachment(attachableValue, named: "loremipsum")
205+
206+
#expect(attachment.attachableValue is MySendableAttachableWithRefcountedSemantics)
207+
#expect(attachment.attachableValue.estimatedAttachmentByteCount == attachableValue.estimatedAttachmentByteCount)
208+
try attachment.attachableValue.withUnsafeBufferPointer(for: attachment) { buffer in
209+
withUnsafeBytes(of: (1, 2, 3)) { buffer2 in
210+
#expect(buffer.elementsEqual(buffer2))
211+
}
212+
}
213+
}
214+
215+
@Test func attachSmallPODValue() throws {
216+
let attachableValue = MySendableAttachableWithSmallPODSemantics(values: (1, 2, 3))
217+
let attachment = Test.Attachment(attachableValue, named: "loremipsum")
218+
219+
#expect(!(type(of: attachment.attachableValue) is AnyClass))
220+
#expect(attachment.attachableValue.estimatedAttachmentByteCount == attachableValue.estimatedAttachmentByteCount)
221+
try attachment.attachableValue.withUnsafeBufferPointer(for: attachment) { buffer in
222+
withUnsafeBytes(of: (1, 2, 3)) { buffer2 in
223+
#expect(buffer.elementsEqual(buffer2))
224+
}
225+
}
226+
}
227+
228+
@Test func attachBigPODValue() throws {
229+
let attachableValue = MySendableAttachableWithBigPODSemantics(values: (1, 2, 3, 4, 5, 6))
230+
let attachment = Test.Attachment(attachableValue, named: "loremipsum")
231+
232+
#expect(type(of: attachment.attachableValue) is AnyClass)
233+
#expect(attachment.attachableValue.estimatedAttachmentByteCount == attachableValue.estimatedAttachmentByteCount)
234+
try attachment.attachableValue.withUnsafeBufferPointer(for: attachment) { buffer in
235+
withUnsafeBytes(of: (1, 2, 3, 4, 5, 6)) { buffer2 in
236+
#expect(buffer.elementsEqual(buffer2))
237+
}
238+
}
239+
}
240+
241+
@Test func attachNonPODValue() throws {
242+
let attachableValue = MySendableAttachableWithNonPODSemantics(values: (1, 2, 3))
243+
let attachment = Test.Attachment(attachableValue, named: "loremipsum")
244+
245+
#expect(type(of: attachment.attachableValue) is AnyClass)
246+
#expect(attachment.attachableValue.estimatedAttachmentByteCount == attachableValue.estimatedAttachmentByteCount)
247+
try attachment.attachableValue.withUnsafeBufferPointer(for: attachment) { buffer in
248+
withUnsafeBytes(of: (1, 2, 3)) { buffer2 in
249+
#expect(buffer.elementsEqual(buffer2))
250+
}
251+
}
252+
}
253+
202254
@Test func issueRecordedWhenAttachingNonSendableValueThatThrows() async {
203255
await confirmation("Attachment detected") { valueAttached in
204256
await confirmation("Issue recorded") { issueRecorded in
@@ -322,13 +374,39 @@ struct MySendableAttachable: Test.Attachable, Sendable {
322374
}
323375
}
324376

325-
struct MySendableAttachableWithDefaultByteCount: Test.Attachable, Sendable {
326-
var string: String
377+
final class MySendableAttachableWithRefcountedSemantics: Test.Attachable, Sendable {
378+
let values: (Int, Int, Int)
379+
380+
init(values: (Int, Int, Int)) {
381+
self.values = values
382+
}
327383

328384
func withUnsafeBufferPointer<R>(for attachment: borrowing Testing.Test.Attachment, _ body: (UnsafeRawBufferPointer) throws -> R) throws -> R {
329-
var string = string
330-
return try string.withUTF8 { buffer in
331-
try body(.init(buffer))
332-
}
385+
try withUnsafeBytes(of: values, body)
386+
}
387+
}
388+
389+
struct MySendableAttachableWithSmallPODSemantics: Test.Attachable, Sendable {
390+
var values: (Int, Int, Int)
391+
392+
func withUnsafeBufferPointer<R>(for attachment: borrowing Testing.Test.Attachment, _ body: (UnsafeRawBufferPointer) throws -> R) throws -> R {
393+
try withUnsafeBytes(of: values, body)
394+
}
395+
}
396+
397+
struct MySendableAttachableWithBigPODSemantics: Test.Attachable, Sendable {
398+
var values: (Int, Int, Int, Int, Int, Int)
399+
400+
func withUnsafeBufferPointer<R>(for attachment: borrowing Testing.Test.Attachment, _ body: (UnsafeRawBufferPointer) throws -> R) throws -> R {
401+
try withUnsafeBytes(of: values, body)
402+
}
403+
}
404+
405+
struct MySendableAttachableWithNonPODSemantics: Test.Attachable, Sendable {
406+
var values: (Int, Int, Int)
407+
var makeItComplicated = "it's complicated"
408+
409+
func withUnsafeBufferPointer<R>(for attachment: borrowing Testing.Test.Attachment, _ body: (UnsafeRawBufferPointer) throws -> R) throws -> R {
410+
try withUnsafeBytes(of: values, body)
333411
}
334412
}

0 commit comments

Comments
 (0)