swiftlang / swift-corelibs-foundation

The Foundation Project, providing core utilities, internationalization, and OS independence
swift.org
Apache License 2.0
5.3k stars 1.14k forks source link

[SR-5501] JSONEncoder leaks memory #4275

Closed swift-ci closed 4 months ago

swift-ci commented 7 years ago
Previous ID SR-5501
Radar rdar://33399116
Original Reporter palle (JIRA User)
Type Bug
Environment Apple Swift version 4.0 (swiftlang-900.0.52 clang-900.0.29) Target: x86_64-apple-macosx10.9
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 5 | |Component/s | Foundation | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: ef4138c621d5277e253566ec7f20a9a3

Issue Description:

An unbounded memory growth of approximately 500MB/s is observed when executing the following code:

import Foundation

struct Test: Codable {
    let foo: String
}

while true {
    let t = Test(foo: "Hello World")
    let encoder = JSONEncoder()
    _ = try encoder.encode(t)   
}

Tested with swiftc Test.swift and swiftc -O -whole-module-optimization Test.swift.

Allocations seem to happen in -[_NSJSONWriter dataWithRootObject:options:error:] and -[_NSJSONWriter resizeTemporaryBuffer:].

swift-ci commented 7 years ago

Comment by LinQingmo (JIRA)

struct Test: Codable {
    let foo: String
}

while true {
    try autoreleasepool {
        let t = Test(foo: "Hello World")
        let encoder = JSONEncoder()
        _ = try encoder.encode(t)
    }
}

Use autoreleasepool, then memory will not grow up.

swift-ci commented 5 years ago

Comment by Nehal Sanklecha (JIRA)

palle (JIRA User) linqingmo (JIRA User)
try autoreleasepool {...) doesn't solve my issue.
Memory is increasing at rapid rate and eventually leads to crash especially on device like iPAD mini 16 GB.
Do we have any other solution ?

swift-ci commented 5 years ago

Comment by Nevyn Bengtsson (JIRA)

I just ran into this issue myself and autoreleasepool{} (INNER around allocation, not around the outside of the while loop) did fix the issue (as it should; the ObjC method in the stack trace is an autoreleasing method). I don't think this is a bug in Swift.

swift-ci commented 5 years ago

Comment by Mark Murphy (JIRA)

Both encode and decode still suffer from this (Xcode 11.0, build 11A420a) and Codable performance has really regressed since moving to Xcode 11. Still working to get my app not to die due to memory leak. Removing either autoreleasepool below triggers leak.

import Foundation

struct Test: Codable {
    let foo: String
}

let t = Test(foo: "Hello World")
let encoder = JSONEncoder()
let decoder = JSONDecoder()

while true {
    var output = Data()
    try autoreleasepool {
        output = try encoder.encode(t)
    }
    var output2 = Test(foo: "Hello World")
    try autoreleasepool {
        output2 = try decoder.decode(Test.self, from: output)
    }
}
swift-ci commented 3 years ago

Comment by Patrik Sjöberg (JIRA)

Any ObjectiveC method (except +new and -init) that returns an object will leak.

ObjectiveC's reference counting is designed to leak objects into autorelease pools. These objects are released when the autoreleasepool is drained. So when running tight loops that at some stage calls ObjectiveC methods you still need to add an autoreleasepool inside that loop, just as you would do in ObjectiveC.

https://developer.apple.com/documentation/foundation/nsautoreleasepool

https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/MemoryMgmt/Articles/mmAutoreleasePools.html#//apple_ref/doc/uid/20000047-CJBFBEDI

krzyzanowskim commented 3 years ago

> So when running tight loops that at some stage calls ObjectiveC methods you still need to add an autoreleasepool inside that loop, just as you would do in ObjectiveC.

I believe the issue here is not that the workaround exists, but that this workaround shouldn't be necessary at this level, preferably.

JSONEncoder is part of Swift Foundation that is multiplatform, while swift `autoreleasepool` is available only on Apple platforms where bridged to ObjC Foundation. Given that, the workaround is cumbersome to apply to the multiplatform codebases. ref: https://forums.swift.org/t/autoreleasepool-for-ubuntu/4419

corymosiman12 commented 1 year ago

I recently ran this in both a macOS and Swift 5.7 docker container here using the new package-benchmark and was not able to reproduce, so I'm not sure this is still a bug. Results below. The memory resident peak should remain constant from p0 -> p100 (refer to discussion here)

Docker

Benchmark results
============================================================================================================================

Host 'e8497bfb8894' with 5 'aarch64' processors with 9 GB memory, running:
#1 SMP PREEMPT Tue Sep 13 07:51:32 UTC 2022

SwiftCoreLibsIssue4275Reproducer
============================================================================================================================

SwiftCoreLibsIssue4275Reproducer
╒══════════════════════════════════════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╕
│ Metric                                   │      p0 │     p25 │     p50 │     p75 │     p90 │     p99 │    p100 │ Samples │
╞══════════════════════════════════════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╡
│ Malloc (large)                           │       0 │       0 │       0 │       0 │       0 │       0 │       0 │    1000 │
├──────────────────────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Malloc (small) (K)                       │      10 │      10 │      10 │      10 │      10 │      10 │      10 │    1000 │
├──────────────────────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Malloc (total) (K)                       │      10 │      10 │      10 │      10 │      10 │      10 │      10 │    1000 │
├──────────────────────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Malloc / free Δ                          │       0 │       0 │       0 │       0 │       0 │       0 │       0 │    1000 │
├──────────────────────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Memory (allocated) (M)                   │      10 │      10 │      10 │      10 │      10 │      10 │      10 │    1000 │
├──────────────────────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Memory (resident peak) (M)               │      20 │      20 │      20 │      20 │      20 │      20 │      20 │    1000 │
├──────────────────────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Memory (virtual peak) (M)                │     106 │     106 │     106 │     106 │     106 │     106 │     106 │    1000 │
╘══════════════════════════════════════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╛

macOS


Benchmark results
============================================================================================================================

Host 'PL- Cory Mosiman KX7157JKXW' with 10 'arm64' processors with 32 GB memory, running:
Darwin Kernel Version 21.6.0: Sun Nov  6 23:31:13 PST 2022; root:xnu-8020.240.14~1/RELEASE_ARM64_T6000SwiftCoreLibsIssue4275Reproducer
============================================================================================================================

SwiftCoreLibsIssue4275Reproducer
╒══════════════════════════════════════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╕
│ Metric                                   │      p0 │     p25 │     p50 │     p75 │     p90 │     p99 │    p100 │ Samples │
╞══════════════════════════════════════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╡
│ Malloc (large)                           │       0 │       0 │       0 │       0 │       0 │       0 │       0 │     688 │
├──────────────────────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Malloc (small) (K)                       │      13 │      13 │      13 │      13 │      13 │      13 │      13 │     688 │
├──────────────────────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Malloc (total) (K)                       │      13 │      13 │      13 │      13 │      13 │      13 │      13 │     688 │
├──────────────────────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Malloc / free Δ                          │       0 │       0 │       0 │       0 │       0 │       0 │       0 │     688 │
├──────────────────────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Memory (allocated) (K)                   │    5620 │    5620 │    5620 │    5620 │    5620 │    5620 │    5620 │     688 │
├──────────────────────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Memory (resident peak) (K)               │    7930 │    7930 │    7930 │    7930 │    7930 │    7930 │    7930 │     688 │
├──────────────────────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Memory (virtual peak) (G)                │     418 │     418 │     418 │     418 │     418 │     418 │     418 │     688 │
╘══════════════════════════════════════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╛
parkera commented 4 months ago

JSONEncoder is now implemented in Swift, so the autoreleasepool requirement should be gone.