From c99a34368d71181b20e342df6cbade17140a886e Mon Sep 17 00:00:00 2001 From: Elijah Semyonov Date: Thu, 26 Sep 2024 22:55:10 +0200 Subject: [PATCH 1/6] Fix things --- ...53914494-654F-4AE6-BFE1-6777D0E5FBA3.plist | 6 +- Sources/SwiftGodot/Core/GArray.swift | 13 +- Sources/SwiftGodot/Variant.swift | 3 +- Tests/SwiftGodotTests/EncodingTests.swift | 393 ++++++++++++++++++ Tests/SwiftGodotTests/MarshalTests.swift | 6 +- Tests/SwiftGodotTests/MemoryLeakTests.swift | 92 ++-- 6 files changed, 472 insertions(+), 41 deletions(-) create mode 100644 Tests/SwiftGodotTests/EncodingTests.swift diff --git a/.swiftpm/xcode/xcshareddata/xcbaselines/SwiftGodotTests.xcbaseline/53914494-654F-4AE6-BFE1-6777D0E5FBA3.plist b/.swiftpm/xcode/xcshareddata/xcbaselines/SwiftGodotTests.xcbaseline/53914494-654F-4AE6-BFE1-6777D0E5FBA3.plist index 84f3981ac..448d781db 100644 --- a/.swiftpm/xcode/xcshareddata/xcbaselines/SwiftGodotTests.xcbaseline/53914494-654F-4AE6-BFE1-6777D0E5FBA3.plist +++ b/.swiftpm/xcode/xcshareddata/xcbaselines/SwiftGodotTests.xcbaseline/53914494-654F-4AE6-BFE1-6777D0E5FBA3.plist @@ -11,7 +11,7 @@ com.apple.XCTPerformanceMetric_WallClockTime baselineAverage - 2.190000 + 0.223000 baselineIntegrationDisplayName Local Baseline @@ -21,7 +21,7 @@ com.apple.XCTPerformanceMetric_WallClockTime baselineAverage - 4.504656 + 0.446000 baselineIntegrationDisplayName Local Baseline @@ -31,7 +31,7 @@ com.apple.XCTPerformanceMetric_WallClockTime baselineAverage - 3.270000 + 0.332172 baselineIntegrationDisplayName Local Baseline diff --git a/Sources/SwiftGodot/Core/GArray.swift b/Sources/SwiftGodot/Core/GArray.swift index d55a93544..bcda7f665 100644 --- a/Sources/SwiftGodot/Core/GArray.swift +++ b/Sources/SwiftGodot/Core/GArray.swift @@ -28,6 +28,8 @@ extension GArray { return Variant() } let ptr = ret.assumingMemoryBound(to: Variant.ContentType.self) + + // We are making a copy of the variant managed by the array. Array is managing its copy, we are managing ours return Variant(copying: ptr.pointee) } set { @@ -35,7 +37,16 @@ extension GArray { return } let ptr = ret.assumingMemoryBound(to: Variant.ContentType.self) - ptr.pointee = newValue.content + + guard ptr.pointee != newValue.content else { + return + } + + // We are taking the variant from the array at the `index` and assuming control over it. Since we don't need it, we just destroy it + gi.variant_destroy(ptr) + + // We are giving array a copy of `newValue` Variant to manage + gi.variant_new_copy(ptr, &newValue.content) } } diff --git a/Sources/SwiftGodot/Variant.swift b/Sources/SwiftGodot/Variant.swift index dfa0e61fb..3076c9ac9 100644 --- a/Sources/SwiftGodot/Variant.swift +++ b/Sources/SwiftGodot/Variant.swift @@ -273,7 +273,8 @@ public class Variant: Hashable, Equatable, CustomDebugStringConvertible { if valid == 0 || oob != 0 { return nil } - return Variant(copying: _result) + + return Variant(takingOver: _result) } set { guard let newValue else { diff --git a/Tests/SwiftGodotTests/EncodingTests.swift b/Tests/SwiftGodotTests/EncodingTests.swift new file mode 100644 index 000000000..bea016846 --- /dev/null +++ b/Tests/SwiftGodotTests/EncodingTests.swift @@ -0,0 +1,393 @@ +// +// EncodingTests.swift +// SwiftGodot +// +// Created by Elijah Semyonov on 26/09/2024. +// + +import XCTest +import SwiftGodotTestability +import SwiftGodot + +struct Foo: Codable { + var myInt: Int + var myText: String + var myIntArray: [Int] +} + +struct FooN: Codable { + var myInt: Int + var myText: String + var myFoo: [Foo] +} + +protocol GodotEncodingContainer { + var data: Variant { get } +} + +extension Vector2: Codable { + enum CodingKeys: String, CodingKey { + case x + case y + } + + public init (from decoder: Decoder) throws { + let values = try decoder.container(keyedBy: CodingKeys.self) + let x = try values.decode(Float.self, forKey: .x) + let y = try values.decode(Float.self, forKey: .y) + self.init (x: x, y: y) + } + + public func encode(to encoder: any Encoder) throws { + var container = encoder.container(keyedBy: CodingKeys.self) + try container.encode(x, forKey: .x) + try container.encode(x, forKey: .y) + } +} + +/// Notes on encoding: +/// - Int8 is encoded as an Int +class GodotEncoder: Encoder { + var codingPath: [any CodingKey] = [] + var userInfo: [CodingUserInfoKey : Any] = [:] + var container: GodotEncodingContainer? = nil + + init () { + + } + + func container(keyedBy type: Key.Type) -> KeyedEncodingContainer where Key : CodingKey { + + var container = GodotKeyedContainer(codingPath: codingPath, userInfo: userInfo) + self.container = container + return KeyedEncodingContainer(container) + } + + var data: Variant { + return container?.data ?? Variant() + } + + func encode(key codingKey: [CodingKey], value: Variant) { + let key = codingKey.map { $0.stringValue }.joined(separator: ".") + fatalError() + //dict [key] = value + } + + func unkeyedContainer() -> any UnkeyedEncodingContainer { + let container = GodotUnkeyedContainer(codingPath: codingPath, userInfo: userInfo) + self.container = container + return container + } + + func singleValueContainer() -> any SingleValueEncodingContainer { + let container = GodotSingleValueContainer(codingPath: codingPath, userInfo: userInfo) + self.container = container + return container + } + + class GodotKeyedContainer: KeyedEncodingContainerProtocol, GodotEncodingContainer { + func encodeNil(forKey key: Key) throws { + var container = self.nestedSingleValueContainer(forKey: key) + fatalError() + //try container.encode(Variant()) + } + + var codingPath: [any CodingKey] = [] + var userInfo: [CodingUserInfoKey: Any] + var storage: [String:GodotEncodingContainer] = [:] + + var data: Variant { + let dict = GDictionary() + for (k,v) in storage { + dict[k] = Variant(v.data) + } + return Variant(dict) + } + + init (codingPath: [any CodingKey], userInfo: [CodingUserInfoKey: Any]) { + self.codingPath = codingPath + self.userInfo = userInfo + } + + func encode(_ value: String, forKey key: Key) throws { + var container = self.nestedSingleValueContainer(forKey: key) + try container.encode(value) + self.storage[key.stringValue] = container + } + + func encode(_ value: Int, forKey key: Key) throws { + let container = self.nestedSingleValueContainer(forKey: key) + try container.encode(value) + self.storage[key.stringValue] = container + } + + func nestedSingleValueContainer(forKey key: Key) + -> GodotSingleValueContainer + { + let container = GodotSingleValueContainer( + codingPath: self.codingPath + [key], + userInfo: self.userInfo + ) + return container + } + + func encode(_ value: T, forKey key: Key) throws where T : Encodable { + let container = self.nestedSingleValueContainer(forKey: key) + try container.encode(value) + self.storage[key.stringValue] = container + + } + + func nestedContainer(keyedBy keyType: NestedKey.Type, forKey key: Key) -> KeyedEncodingContainer where NestedKey : CodingKey { + fatalError() + } + + func nestedUnkeyedContainer(forKey key: Key) -> any UnkeyedEncodingContainer { + fatalError() + } + + func superEncoder() -> any Encoder { + fatalError() + } + + func superEncoder(forKey key: Key) -> any Encoder { + fatalError() + } + } + + class GodotUnkeyedContainer: UnkeyedEncodingContainer, GodotEncodingContainer { + var codingPath: [any CodingKey] = [] + var userInfo: [CodingUserInfoKey: Any] + + var storage = GArray() + + var data: Variant { + return Variant(storage) + } + + init (codingPath: [any CodingKey], userInfo: [CodingUserInfoKey: Any]) { + self.codingPath = codingPath + self.userInfo = userInfo + } + + var count: Int { + Int(storage.size()) + } + + func encode(_ value: String) throws { + fatalError() + } + + func encode(_ value: Double) throws { + fatalError() + } + + func encode(_ value: Float) throws { + fatalError() + } + + func encode(_ value: Int) throws { + fatalError() + } + + func encode(_ value: Int8) throws { + fatalError() + } + + func encode(_ value: Int16) throws { + fatalError() + } + + func encode(_ value: Int32) throws { + fatalError() + } + + func encode(_ value: Int64) throws { + fatalError() + } + + func encode(_ value: UInt) throws { + fatalError() + } + + func encode(_ value: UInt8) throws { + fatalError() + } + + func encode(_ value: UInt16) throws { + fatalError() + } + + func encode(_ value: UInt32) throws { + fatalError() + } + + func encode(_ value: UInt64) throws { + fatalError() + } + + func encode(_ value: T) throws where T: Encodable { + let base = Int(storage.size()) + + + if let v = value as? Array { + _ = storage.resize(size: storage.size () + Int64(v.count)) + for i in 0..(keyedBy keyType: NestedKey.Type) -> KeyedEncodingContainer where NestedKey : CodingKey { + fatalError() + } + + func nestedUnkeyedContainer() -> any UnkeyedEncodingContainer { + fatalError() + } + + func superEncoder() -> any Encoder { + fatalError() + } + + + } + + class GodotSingleValueContainer: SingleValueEncodingContainer, GodotEncodingContainer { + func encode(_ value: T) throws where T : Encodable { + if value is Array { + var container = GodotUnkeyedContainer (codingPath: codingPath, userInfo: userInfo) + try container.encode(value) + self.value = container.data + } else if let i = value as? Int { + try self.encode(i) + } else if let str = value as? String { + try self.encode(str) + } else { + var nested = GodotEncoder() + try value.encode(to: nested) + self.value = nested.container?.data + } + } + + enum foo: Error { + case nope + } + + var codingPath: [any CodingKey] = [] + var userInfo: [CodingUserInfoKey: Any] + var value: Variant? + + var data: Variant { + value ?? Variant() + } + + init (codingPath: [any CodingKey], userInfo: [CodingUserInfoKey: Any]) { + self.codingPath = codingPath + self.userInfo = userInfo + } + + func encodeNil() throws { + fatalError() + } + + func encode(_ value: Bool) throws { + fatalError() + } + + func encode(_ value: String) throws { + self.value = Variant(value) + } + + func encode(_ value: Double) throws { + fatalError() + } + + func encode(_ value: Float) throws { + fatalError() + } + + func encode(_ value: Int) throws { + self.value = Variant(Int(value)) + } + + func encode(_ value: Int8) throws { + fatalError() + } + + func encode(_ value: Int16) throws { + fatalError() + } + + func encode(_ value: Int32) throws { + fatalError() + } + + func encode(_ value: Int64) throws { + fatalError() + } + + func encode(_ value: UInt) throws { + fatalError() + } + + func encode(_ value: UInt8) throws { + fatalError() + } + + func encode(_ value: UInt16) throws { + fatalError() + } + + func encode(_ value: UInt32) throws { + fatalError() + } + + func encode(_ value: UInt64) throws { + fatalError() + } + } +} + + +final class EncodingTests: GodotTestCase { + func testEncoding() { + let g = GodotEncoder() + let foon = FooN(myInt: 9, myText: "nine", myFoo: [ + Foo(myInt: 10, myText: "Nested1", myIntArray: [1,1,1]), + Foo(myInt: 30, myText: "Nested2", myIntArray: [2,2,2])]) + try? foon.encode (to:g) + + print(g.data) + } +} diff --git a/Tests/SwiftGodotTests/MarshalTests.swift b/Tests/SwiftGodotTests/MarshalTests.swift index af2dfe807..a5aae5537 100644 --- a/Tests/SwiftGodotTests/MarshalTests.swift +++ b/Tests/SwiftGodotTests/MarshalTests.swift @@ -39,7 +39,7 @@ final class MarshalTests: GodotTestCase { let child = TestVariant() measure { - for _ in 0..<10000000 { + for _ in 0..<1_000_000 { tv.addChild(node: child) tv.removeChild(node: child) } @@ -58,7 +58,7 @@ final class MarshalTests: GodotTestCase { let weight = 0.23 measure { - for _ in 0..<10000000 { + for _ in 0..<1_000_000 { let _ = a.cubicInterpolate(b: b, preA: preA, postB: preB, weight: weight) } } @@ -68,7 +68,7 @@ final class MarshalTests: GodotTestCase { let randomValues = (0..<10).map { _ in Variant(Float.random(in: -1.0...1.0)) } measure { - for _ in 0..<1000000 { + for _ in 0..<100_000 { let _ = GD.max(arg1: randomValues[0], arg2: randomValues[1], randomValues[2], randomValues[3], randomValues[4], randomValues[5], randomValues[6], randomValues[7], randomValues[8], randomValues[9]) } } diff --git a/Tests/SwiftGodotTests/MemoryLeakTests.swift b/Tests/SwiftGodotTests/MemoryLeakTests.swift index f4ded5417..133283954 100644 --- a/Tests/SwiftGodotTests/MemoryLeakTests.swift +++ b/Tests/SwiftGodotTests/MemoryLeakTests.swift @@ -3,6 +3,13 @@ import SwiftGodotTestability import XCTest final class MemoryLeakTests: GodotTestCase { + func checkLeaks(_ body: () -> Void) { + let before = Performance.getMonitor(.memoryStatic) + body() + let after = Performance.getMonitor(.memoryStatic) + + XCTAssertEqual(before, after, "Leaked \(after - before) bytes") + } // https://github.com/migueldeicaza/SwiftGodot/issues/513 func test_513_leak1() { @@ -18,17 +25,12 @@ final class MemoryLeakTests: GodotTestCase { // Warm-up the code path in case it performs any one-time permanent allocations. oneIteration(object: object) - - let before = Performance.getMonitor(.memoryStatic) - let count = 1_000 - - for _ in 0 ..< count { - oneIteration(object: object) + + checkLeaks { + for _ in 0 ..< 1_000 { + oneIteration(object: object) + } } - - let after = Performance.getMonitor(.memoryStatic) - - XCTAssertEqual(before, after, "Leaked \(Int((after - before) / Double(count))) bytes per iteration.") } // https://github.com/migueldeicaza/SwiftGodot/issues/513 @@ -48,31 +50,20 @@ final class MemoryLeakTests: GodotTestCase { // Warm-up the code path in case it performs any one-time permanent allocations. oneIteration(bytes: bytes) - let before = Performance.getMonitor(.memoryStatic) - let count = 1_000 - - for _ in 0 ..< count { - oneIteration(bytes: bytes) + checkLeaks { + for _ in 0 ..< 1_000 { + oneIteration(bytes: bytes) + } } - - let after = Performance.getMonitor(.memoryStatic) - - XCTAssertEqual(before, after, "Leaked \(Int((after - before) / Double(count))) bytes per iteration.") } // https://github.com/migueldeicaza/SwiftGodot/issues/541 func test_541_leak() { - let before = Performance.getMonitor(.memoryStatic) - - for _ in 0...10000000 { - autoreleasepool { + checkLeaks { + for _ in 0...10_000_000 { let _ = Variant("daosdoasodasoda") } } - - let after = Performance.getMonitor(.memoryStatic) - - XCTAssertEqual(before, after, "Leaked \(Int(after - before))") } // https://github.com/migueldeicaza/SwiftGodot/issues/544 @@ -83,14 +74,49 @@ final class MemoryLeakTests: GodotTestCase { // https://docs.godotengine.org/en/stable/classes/class_string.html#class-string-method-left let methodName = StringName("left") - let before = Performance.getMonitor(.memoryStatic) + checkLeaks { + for _ in 0 ..< 2_000 { + let _ = variant.call(method: methodName, Variant(2)) + } + } + } + + // https://github.com/migueldeicaza/SwiftGodot/issues/543 + func test_543_leak() { + let string = "Hello, World!" + let variant = Variant(string) + + checkLeaks { + for _ in 0 ..< 2000 { + _ = variant[0] + } + } + } + + func test_array_leaks() { + let array = GArray() + array.append(Variant("S")) + array.append(Variant("M")) - for _ in 0 ..< 2000 { - let _ = variant.call(method: methodName, Variant(2)) + checkLeaks { + for _ in 0 ..< 1_000 { + array[0] = Variant("T") + _ = array[1] + } } - let after = Performance.getMonitor(.memoryStatic) - - XCTAssertEqual(before, after, "Leaked \(Int(after - before)) bytes") + XCTAssertEqual(array[0], Variant("T")) + + let variant = Variant(array) + + checkLeaks { + for _ in 0 ..< 1_000 { + variant[0] = Variant("U") + _ = variant[1] + variant[1] = Variant("K") + } + } + + XCTAssertEqual(variant[0], Variant("U")) } } From 6bf0d1abec8e8f3c34f18ac19e35f43cf10313a4 Mon Sep 17 00:00:00 2001 From: Elijah Semyonov Date: Thu, 26 Sep 2024 23:05:01 +0200 Subject: [PATCH 2/6] Augment tests --- Tests/SwiftGodotTests/MemoryLeakTests.swift | 24 ++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/Tests/SwiftGodotTests/MemoryLeakTests.swift b/Tests/SwiftGodotTests/MemoryLeakTests.swift index 133283954..6d07bab72 100644 --- a/Tests/SwiftGodotTests/MemoryLeakTests.swift +++ b/Tests/SwiftGodotTests/MemoryLeakTests.swift @@ -3,12 +3,25 @@ import SwiftGodotTestability import XCTest final class MemoryLeakTests: GodotTestCase { - func checkLeaks(_ body: () -> Void) { + /// Check that `body` doesn't leak. Or ensure that something is leaking, if `useUnoReverseCard` is true + func checkLeaks(useUnoReverseCard: Bool = false, _ body: () -> Void) { let before = Performance.getMonitor(.memoryStatic) body() let after = Performance.getMonitor(.memoryStatic) - XCTAssertEqual(before, after, "Leaked \(after - before) bytes") + if useUnoReverseCard { + XCTAssertNotEqual(before, after, "It should leak!") + } else { + XCTAssertEqual(before, after, "Leaked \(after - before) bytes") + } + } + + func testThatItLeaksIndeed() { + let array = GArray() + + checkLeaks(useUnoReverseCard: true) { + array.append(Variant(10)) + } } // https://github.com/migueldeicaza/SwiftGodot/issues/513 @@ -99,6 +112,8 @@ final class MemoryLeakTests: GodotTestCase { array.append(Variant("M")) checkLeaks { + XCTAssertEqual(array[0], Variant("S")) + for _ in 0 ..< 1_000 { array[0] = Variant("T") _ = array[1] @@ -110,11 +125,14 @@ final class MemoryLeakTests: GodotTestCase { let variant = Variant(array) checkLeaks { + XCTAssertEqual(variant[0], Variant("T")) + for _ in 0 ..< 1_000 { variant[0] = Variant("U") - _ = variant[1] variant[1] = Variant("K") } + + XCTAssertEqual(variant[2], Variant("U")) } XCTAssertEqual(variant[0], Variant("U")) From af715a1435a2fb6ab21a1d5becf60046af21f6a0 Mon Sep 17 00:00:00 2001 From: Elijah Semyonov Date: Thu, 26 Sep 2024 23:08:59 +0200 Subject: [PATCH 3/6] Minor fixes --- Tests/SwiftGodotTests/EncodingTests.swift | 2 +- Tests/SwiftGodotTests/MemoryLeakTests.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/SwiftGodotTests/EncodingTests.swift b/Tests/SwiftGodotTests/EncodingTests.swift index bea016846..aa9388d07 100644 --- a/Tests/SwiftGodotTests/EncodingTests.swift +++ b/Tests/SwiftGodotTests/EncodingTests.swift @@ -388,6 +388,6 @@ final class EncodingTests: GodotTestCase { Foo(myInt: 30, myText: "Nested2", myIntArray: [2,2,2])]) try? foon.encode (to:g) - print(g.data) + print(g.data.description) } } diff --git a/Tests/SwiftGodotTests/MemoryLeakTests.swift b/Tests/SwiftGodotTests/MemoryLeakTests.swift index 6d07bab72..88624f7a0 100644 --- a/Tests/SwiftGodotTests/MemoryLeakTests.swift +++ b/Tests/SwiftGodotTests/MemoryLeakTests.swift @@ -132,7 +132,7 @@ final class MemoryLeakTests: GodotTestCase { variant[1] = Variant("K") } - XCTAssertEqual(variant[2], Variant("U")) + XCTAssertEqual(variant[1], Variant("K")) } XCTAssertEqual(variant[0], Variant("U")) From a94b0d565dd1f5452a8b8618de5c2d51db12e4d5 Mon Sep 17 00:00:00 2001 From: Elijah Semyonov Date: Thu, 26 Sep 2024 23:12:32 +0200 Subject: [PATCH 4/6] Rename a test and add a link --- Tests/SwiftGodotTests/EncodingTests.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tests/SwiftGodotTests/EncodingTests.swift b/Tests/SwiftGodotTests/EncodingTests.swift index aa9388d07..b4055e977 100644 --- a/Tests/SwiftGodotTests/EncodingTests.swift +++ b/Tests/SwiftGodotTests/EncodingTests.swift @@ -381,7 +381,8 @@ class GodotEncoder: Encoder { final class EncodingTests: GodotTestCase { - func testEncoding() { + // https://github.com/migueldeicaza/SwiftGodot/issues/531 + func test_531_crash() { let g = GodotEncoder() let foon = FooN(myInt: 9, myText: "nine", myFoo: [ Foo(myInt: 10, myText: "Nested1", myIntArray: [1,1,1]), From c7ef792404072bda11b7f509509b12bc00c36bbe Mon Sep 17 00:00:00 2001 From: Elijah Semyonov Date: Thu, 26 Sep 2024 23:15:06 +0200 Subject: [PATCH 5/6] Slap a leak check as an extra and throw a towel on it --- Tests/SwiftGodotTests/EncodingTests.swift | 394 -------------------- Tests/SwiftGodotTests/MemoryLeakTests.swift | 382 +++++++++++++++++++ 2 files changed, 382 insertions(+), 394 deletions(-) delete mode 100644 Tests/SwiftGodotTests/EncodingTests.swift diff --git a/Tests/SwiftGodotTests/EncodingTests.swift b/Tests/SwiftGodotTests/EncodingTests.swift deleted file mode 100644 index b4055e977..000000000 --- a/Tests/SwiftGodotTests/EncodingTests.swift +++ /dev/null @@ -1,394 +0,0 @@ -// -// EncodingTests.swift -// SwiftGodot -// -// Created by Elijah Semyonov on 26/09/2024. -// - -import XCTest -import SwiftGodotTestability -import SwiftGodot - -struct Foo: Codable { - var myInt: Int - var myText: String - var myIntArray: [Int] -} - -struct FooN: Codable { - var myInt: Int - var myText: String - var myFoo: [Foo] -} - -protocol GodotEncodingContainer { - var data: Variant { get } -} - -extension Vector2: Codable { - enum CodingKeys: String, CodingKey { - case x - case y - } - - public init (from decoder: Decoder) throws { - let values = try decoder.container(keyedBy: CodingKeys.self) - let x = try values.decode(Float.self, forKey: .x) - let y = try values.decode(Float.self, forKey: .y) - self.init (x: x, y: y) - } - - public func encode(to encoder: any Encoder) throws { - var container = encoder.container(keyedBy: CodingKeys.self) - try container.encode(x, forKey: .x) - try container.encode(x, forKey: .y) - } -} - -/// Notes on encoding: -/// - Int8 is encoded as an Int -class GodotEncoder: Encoder { - var codingPath: [any CodingKey] = [] - var userInfo: [CodingUserInfoKey : Any] = [:] - var container: GodotEncodingContainer? = nil - - init () { - - } - - func container(keyedBy type: Key.Type) -> KeyedEncodingContainer where Key : CodingKey { - - var container = GodotKeyedContainer(codingPath: codingPath, userInfo: userInfo) - self.container = container - return KeyedEncodingContainer(container) - } - - var data: Variant { - return container?.data ?? Variant() - } - - func encode(key codingKey: [CodingKey], value: Variant) { - let key = codingKey.map { $0.stringValue }.joined(separator: ".") - fatalError() - //dict [key] = value - } - - func unkeyedContainer() -> any UnkeyedEncodingContainer { - let container = GodotUnkeyedContainer(codingPath: codingPath, userInfo: userInfo) - self.container = container - return container - } - - func singleValueContainer() -> any SingleValueEncodingContainer { - let container = GodotSingleValueContainer(codingPath: codingPath, userInfo: userInfo) - self.container = container - return container - } - - class GodotKeyedContainer: KeyedEncodingContainerProtocol, GodotEncodingContainer { - func encodeNil(forKey key: Key) throws { - var container = self.nestedSingleValueContainer(forKey: key) - fatalError() - //try container.encode(Variant()) - } - - var codingPath: [any CodingKey] = [] - var userInfo: [CodingUserInfoKey: Any] - var storage: [String:GodotEncodingContainer] = [:] - - var data: Variant { - let dict = GDictionary() - for (k,v) in storage { - dict[k] = Variant(v.data) - } - return Variant(dict) - } - - init (codingPath: [any CodingKey], userInfo: [CodingUserInfoKey: Any]) { - self.codingPath = codingPath - self.userInfo = userInfo - } - - func encode(_ value: String, forKey key: Key) throws { - var container = self.nestedSingleValueContainer(forKey: key) - try container.encode(value) - self.storage[key.stringValue] = container - } - - func encode(_ value: Int, forKey key: Key) throws { - let container = self.nestedSingleValueContainer(forKey: key) - try container.encode(value) - self.storage[key.stringValue] = container - } - - func nestedSingleValueContainer(forKey key: Key) - -> GodotSingleValueContainer - { - let container = GodotSingleValueContainer( - codingPath: self.codingPath + [key], - userInfo: self.userInfo - ) - return container - } - - func encode(_ value: T, forKey key: Key) throws where T : Encodable { - let container = self.nestedSingleValueContainer(forKey: key) - try container.encode(value) - self.storage[key.stringValue] = container - - } - - func nestedContainer(keyedBy keyType: NestedKey.Type, forKey key: Key) -> KeyedEncodingContainer where NestedKey : CodingKey { - fatalError() - } - - func nestedUnkeyedContainer(forKey key: Key) -> any UnkeyedEncodingContainer { - fatalError() - } - - func superEncoder() -> any Encoder { - fatalError() - } - - func superEncoder(forKey key: Key) -> any Encoder { - fatalError() - } - } - - class GodotUnkeyedContainer: UnkeyedEncodingContainer, GodotEncodingContainer { - var codingPath: [any CodingKey] = [] - var userInfo: [CodingUserInfoKey: Any] - - var storage = GArray() - - var data: Variant { - return Variant(storage) - } - - init (codingPath: [any CodingKey], userInfo: [CodingUserInfoKey: Any]) { - self.codingPath = codingPath - self.userInfo = userInfo - } - - var count: Int { - Int(storage.size()) - } - - func encode(_ value: String) throws { - fatalError() - } - - func encode(_ value: Double) throws { - fatalError() - } - - func encode(_ value: Float) throws { - fatalError() - } - - func encode(_ value: Int) throws { - fatalError() - } - - func encode(_ value: Int8) throws { - fatalError() - } - - func encode(_ value: Int16) throws { - fatalError() - } - - func encode(_ value: Int32) throws { - fatalError() - } - - func encode(_ value: Int64) throws { - fatalError() - } - - func encode(_ value: UInt) throws { - fatalError() - } - - func encode(_ value: UInt8) throws { - fatalError() - } - - func encode(_ value: UInt16) throws { - fatalError() - } - - func encode(_ value: UInt32) throws { - fatalError() - } - - func encode(_ value: UInt64) throws { - fatalError() - } - - func encode(_ value: T) throws where T: Encodable { - let base = Int(storage.size()) - - - if let v = value as? Array { - _ = storage.resize(size: storage.size () + Int64(v.count)) - for i in 0..(keyedBy keyType: NestedKey.Type) -> KeyedEncodingContainer where NestedKey : CodingKey { - fatalError() - } - - func nestedUnkeyedContainer() -> any UnkeyedEncodingContainer { - fatalError() - } - - func superEncoder() -> any Encoder { - fatalError() - } - - - } - - class GodotSingleValueContainer: SingleValueEncodingContainer, GodotEncodingContainer { - func encode(_ value: T) throws where T : Encodable { - if value is Array { - var container = GodotUnkeyedContainer (codingPath: codingPath, userInfo: userInfo) - try container.encode(value) - self.value = container.data - } else if let i = value as? Int { - try self.encode(i) - } else if let str = value as? String { - try self.encode(str) - } else { - var nested = GodotEncoder() - try value.encode(to: nested) - self.value = nested.container?.data - } - } - - enum foo: Error { - case nope - } - - var codingPath: [any CodingKey] = [] - var userInfo: [CodingUserInfoKey: Any] - var value: Variant? - - var data: Variant { - value ?? Variant() - } - - init (codingPath: [any CodingKey], userInfo: [CodingUserInfoKey: Any]) { - self.codingPath = codingPath - self.userInfo = userInfo - } - - func encodeNil() throws { - fatalError() - } - - func encode(_ value: Bool) throws { - fatalError() - } - - func encode(_ value: String) throws { - self.value = Variant(value) - } - - func encode(_ value: Double) throws { - fatalError() - } - - func encode(_ value: Float) throws { - fatalError() - } - - func encode(_ value: Int) throws { - self.value = Variant(Int(value)) - } - - func encode(_ value: Int8) throws { - fatalError() - } - - func encode(_ value: Int16) throws { - fatalError() - } - - func encode(_ value: Int32) throws { - fatalError() - } - - func encode(_ value: Int64) throws { - fatalError() - } - - func encode(_ value: UInt) throws { - fatalError() - } - - func encode(_ value: UInt8) throws { - fatalError() - } - - func encode(_ value: UInt16) throws { - fatalError() - } - - func encode(_ value: UInt32) throws { - fatalError() - } - - func encode(_ value: UInt64) throws { - fatalError() - } - } -} - - -final class EncodingTests: GodotTestCase { - // https://github.com/migueldeicaza/SwiftGodot/issues/531 - func test_531_crash() { - let g = GodotEncoder() - let foon = FooN(myInt: 9, myText: "nine", myFoo: [ - Foo(myInt: 10, myText: "Nested1", myIntArray: [1,1,1]), - Foo(myInt: 30, myText: "Nested2", myIntArray: [2,2,2])]) - try? foon.encode (to:g) - - print(g.data.description) - } -} diff --git a/Tests/SwiftGodotTests/MemoryLeakTests.swift b/Tests/SwiftGodotTests/MemoryLeakTests.swift index 88624f7a0..37708726c 100644 --- a/Tests/SwiftGodotTests/MemoryLeakTests.swift +++ b/Tests/SwiftGodotTests/MemoryLeakTests.swift @@ -2,6 +2,376 @@ import SwiftGodot import SwiftGodotTestability import XCTest +struct Foo: Codable { + var myInt: Int + var myText: String + var myIntArray: [Int] +} + +struct FooN: Codable { + var myInt: Int + var myText: String + var myFoo: [Foo] +} + +protocol GodotEncodingContainer { + var data: Variant { get } +} + +extension Vector2: Codable { + enum CodingKeys: String, CodingKey { + case x + case y + } + + public init (from decoder: Decoder) throws { + let values = try decoder.container(keyedBy: CodingKeys.self) + let x = try values.decode(Float.self, forKey: .x) + let y = try values.decode(Float.self, forKey: .y) + self.init (x: x, y: y) + } + + public func encode(to encoder: any Encoder) throws { + var container = encoder.container(keyedBy: CodingKeys.self) + try container.encode(x, forKey: .x) + try container.encode(x, forKey: .y) + } +} + +/// Notes on encoding: +/// - Int8 is encoded as an Int +class GodotEncoder: Encoder { + var codingPath: [any CodingKey] = [] + var userInfo: [CodingUserInfoKey : Any] = [:] + var container: GodotEncodingContainer? = nil + + init () { + + } + + func container(keyedBy type: Key.Type) -> KeyedEncodingContainer where Key : CodingKey { + + var container = GodotKeyedContainer(codingPath: codingPath, userInfo: userInfo) + self.container = container + return KeyedEncodingContainer(container) + } + + var data: Variant { + return container?.data ?? Variant() + } + + func encode(key codingKey: [CodingKey], value: Variant) { + let key = codingKey.map { $0.stringValue }.joined(separator: ".") + fatalError() + //dict [key] = value + } + + func unkeyedContainer() -> any UnkeyedEncodingContainer { + let container = GodotUnkeyedContainer(codingPath: codingPath, userInfo: userInfo) + self.container = container + return container + } + + func singleValueContainer() -> any SingleValueEncodingContainer { + let container = GodotSingleValueContainer(codingPath: codingPath, userInfo: userInfo) + self.container = container + return container + } + + class GodotKeyedContainer: KeyedEncodingContainerProtocol, GodotEncodingContainer { + func encodeNil(forKey key: Key) throws { + var container = self.nestedSingleValueContainer(forKey: key) + fatalError() + //try container.encode(Variant()) + } + + var codingPath: [any CodingKey] = [] + var userInfo: [CodingUserInfoKey: Any] + var storage: [String:GodotEncodingContainer] = [:] + + var data: Variant { + let dict = GDictionary() + for (k,v) in storage { + dict[k] = Variant(v.data) + } + return Variant(dict) + } + + init (codingPath: [any CodingKey], userInfo: [CodingUserInfoKey: Any]) { + self.codingPath = codingPath + self.userInfo = userInfo + } + + func encode(_ value: String, forKey key: Key) throws { + var container = self.nestedSingleValueContainer(forKey: key) + try container.encode(value) + self.storage[key.stringValue] = container + } + + func encode(_ value: Int, forKey key: Key) throws { + let container = self.nestedSingleValueContainer(forKey: key) + try container.encode(value) + self.storage[key.stringValue] = container + } + + func nestedSingleValueContainer(forKey key: Key) + -> GodotSingleValueContainer + { + let container = GodotSingleValueContainer( + codingPath: self.codingPath + [key], + userInfo: self.userInfo + ) + return container + } + + func encode(_ value: T, forKey key: Key) throws where T : Encodable { + let container = self.nestedSingleValueContainer(forKey: key) + try container.encode(value) + self.storage[key.stringValue] = container + + } + + func nestedContainer(keyedBy keyType: NestedKey.Type, forKey key: Key) -> KeyedEncodingContainer where NestedKey : CodingKey { + fatalError() + } + + func nestedUnkeyedContainer(forKey key: Key) -> any UnkeyedEncodingContainer { + fatalError() + } + + func superEncoder() -> any Encoder { + fatalError() + } + + func superEncoder(forKey key: Key) -> any Encoder { + fatalError() + } + } + + class GodotUnkeyedContainer: UnkeyedEncodingContainer, GodotEncodingContainer { + var codingPath: [any CodingKey] = [] + var userInfo: [CodingUserInfoKey: Any] + + var storage = GArray() + + var data: Variant { + return Variant(storage) + } + + init (codingPath: [any CodingKey], userInfo: [CodingUserInfoKey: Any]) { + self.codingPath = codingPath + self.userInfo = userInfo + } + + var count: Int { + Int(storage.size()) + } + + func encode(_ value: String) throws { + fatalError() + } + + func encode(_ value: Double) throws { + fatalError() + } + + func encode(_ value: Float) throws { + fatalError() + } + + func encode(_ value: Int) throws { + fatalError() + } + + func encode(_ value: Int8) throws { + fatalError() + } + + func encode(_ value: Int16) throws { + fatalError() + } + + func encode(_ value: Int32) throws { + fatalError() + } + + func encode(_ value: Int64) throws { + fatalError() + } + + func encode(_ value: UInt) throws { + fatalError() + } + + func encode(_ value: UInt8) throws { + fatalError() + } + + func encode(_ value: UInt16) throws { + fatalError() + } + + func encode(_ value: UInt32) throws { + fatalError() + } + + func encode(_ value: UInt64) throws { + fatalError() + } + + func encode(_ value: T) throws where T: Encodable { + let base = Int(storage.size()) + + + if let v = value as? Array { + _ = storage.resize(size: storage.size () + Int64(v.count)) + for i in 0..(keyedBy keyType: NestedKey.Type) -> KeyedEncodingContainer where NestedKey : CodingKey { + fatalError() + } + + func nestedUnkeyedContainer() -> any UnkeyedEncodingContainer { + fatalError() + } + + func superEncoder() -> any Encoder { + fatalError() + } + + + } + + class GodotSingleValueContainer: SingleValueEncodingContainer, GodotEncodingContainer { + func encode(_ value: T) throws where T : Encodable { + if value is Array { + var container = GodotUnkeyedContainer (codingPath: codingPath, userInfo: userInfo) + try container.encode(value) + self.value = container.data + } else if let i = value as? Int { + try self.encode(i) + } else if let str = value as? String { + try self.encode(str) + } else { + var nested = GodotEncoder() + try value.encode(to: nested) + self.value = nested.container?.data + } + } + + enum foo: Error { + case nope + } + + var codingPath: [any CodingKey] = [] + var userInfo: [CodingUserInfoKey: Any] + var value: Variant? + + var data: Variant { + value ?? Variant() + } + + init (codingPath: [any CodingKey], userInfo: [CodingUserInfoKey: Any]) { + self.codingPath = codingPath + self.userInfo = userInfo + } + + func encodeNil() throws { + fatalError() + } + + func encode(_ value: Bool) throws { + fatalError() + } + + func encode(_ value: String) throws { + self.value = Variant(value) + } + + func encode(_ value: Double) throws { + fatalError() + } + + func encode(_ value: Float) throws { + fatalError() + } + + func encode(_ value: Int) throws { + self.value = Variant(Int(value)) + } + + func encode(_ value: Int8) throws { + fatalError() + } + + func encode(_ value: Int16) throws { + fatalError() + } + + func encode(_ value: Int32) throws { + fatalError() + } + + func encode(_ value: Int64) throws { + fatalError() + } + + func encode(_ value: UInt) throws { + fatalError() + } + + func encode(_ value: UInt8) throws { + fatalError() + } + + func encode(_ value: UInt16) throws { + fatalError() + } + + func encode(_ value: UInt32) throws { + fatalError() + } + + func encode(_ value: UInt64) throws { + fatalError() + } + } +} + final class MemoryLeakTests: GodotTestCase { /// Check that `body` doesn't leak. Or ensure that something is leaking, if `useUnoReverseCard` is true func checkLeaks(useUnoReverseCard: Bool = false, _ body: () -> Void) { @@ -137,4 +507,16 @@ final class MemoryLeakTests: GodotTestCase { XCTAssertEqual(variant[0], Variant("U")) } + + func test_531_crash_or_leak() { + checkLeaks { + let g = GodotEncoder() + let foon = FooN(myInt: 9, myText: "nine", myFoo: [ + Foo(myInt: 10, myText: "Nested1", myIntArray: [1,1,1]), + Foo(myInt: 30, myText: "Nested2", myIntArray: [2,2,2])]) + try? foon.encode(to: g) + + print(g.data.description) + } + } } From ecbe9fde7e46f89e4f030717b29c8c67531c6d25 Mon Sep 17 00:00:00 2001 From: Elijah Semyonov Date: Fri, 27 Sep 2024 09:37:13 +0200 Subject: [PATCH 6/6] Fix crash and leak in GDictionary --- Generator/Generator/BuiltinGen.swift | 38 +++++++++++---------- Tests/SwiftGodotTests/MemoryLeakTests.swift | 26 +++++++++++++- 2 files changed, 45 insertions(+), 19 deletions(-) diff --git a/Generator/Generator/BuiltinGen.swift b/Generator/Generator/BuiltinGen.swift index 714a0df27..0587b76b6 100644 --- a/Generator/Generator/BuiltinGen.swift +++ b/Generator/Generator/BuiltinGen.swift @@ -508,28 +508,30 @@ func generateBuiltinMethods (_ p: Printer, p.staticVar (visibility: "private ", name: "keyed_checker", type: "GDExtensionPtrKeyedChecker") { p ("return gi.variant_get_ptr_keyed_checker (\(variantType))!") } - p ("public subscript (key: Variant) -> Variant?") { - p ("get") { - p ("let keyCopy = key") - p ("var result = Variant.zero") - p ("if Self.keyed_checker (&content, &keyCopy.content) != 0") { - p ("Self.keyed_getter (&content, &keyCopy.content, &result)") - p ("return Variant(copying: result)") - } - p ("else") { - p ("return nil") + p(""" + public subscript(key: Variant) -> Variant? { + get { + var result = Variant.zero + if Self.keyed_checker(&content, &key.content) != 0 { + Self.keyed_getter (&content, &key.content, &result) + // Returns unowned handle + return Variant(takingOver: result) + } else { + return nil } } - p ("set") { - p ("let keyCopy = key") - p ("if let newCopy = newValue") { - p ("Self.keyed_setter (&content, &keyCopy.content, &newCopy.content)") - } - p ("else") { - p ("Self.keyed_setter (&content, &keyCopy.content, nil)") - } + + set { + if let newValue { + Self.keyed_setter(&content, &key.content, &newValue.content) + } else { + var nilContent = Variant.zero + // nil will cause a crash, needs a pointer to Nil Variant content instead + Self.keyed_setter(&content, &key.content, &nilContent) + } } } + """) } if let returnType = bc.indexingReturnType, !bc.isKeyed, !bc.name.hasSuffix ("Array"), bc.name != "String" { let godotType = getGodotType (JGodotReturnValue (type: returnType, meta: nil)) diff --git a/Tests/SwiftGodotTests/MemoryLeakTests.swift b/Tests/SwiftGodotTests/MemoryLeakTests.swift index 37708726c..293ce50eb 100644 --- a/Tests/SwiftGodotTests/MemoryLeakTests.swift +++ b/Tests/SwiftGodotTests/MemoryLeakTests.swift @@ -443,7 +443,7 @@ final class MemoryLeakTests: GodotTestCase { // https://github.com/migueldeicaza/SwiftGodot/issues/541 func test_541_leak() { checkLeaks { - for _ in 0...10_000_000 { + for _ in 0...1_000 { let _ = Variant("daosdoasodasoda") } } @@ -519,4 +519,28 @@ final class MemoryLeakTests: GodotTestCase { print(g.data.description) } } + + func test_dictionary_leaks() { + checkLeaks { + let dictionary = GDictionary() + + for i in 0 ..< 1_000 { + let variant = Variant("value\(i)") + dictionary["key\(i * 2)"] = variant + dictionary["key\(i * 2 + 1)"] = variant + } + + for i in 0 ..< 1_000 { + let key = "key\(Int.random(in: 0 ..< 2_000))" + let value = dictionary[key] + dictionary["key\(i * 2)"] = value + } + + for i in 0 ..< 1_000 { + _ = dictionary.erase(key: Variant("\(i * 2)")) + } + + dictionary.clear() + } + } }