Skip to content
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

Jump to bottom

[SR-11231] ManagedBufferPointer's suggested deinit allocates #53632

Closed
weissi opened this issue Jul 30, 2019 · 5 comments
Closed

[SR-11231] ManagedBufferPointer's suggested deinit allocates #53632

weissi opened this issue Jul 30, 2019 · 5 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. standard library Area: Standard library umbrella

Comments

@weissi
Copy link
Member

weissi commented Jul 30, 2019

Previous ID SR-11231
Radar rdar://problem/53777612
Original Reporter @weissi
Type Bug
Status Resolved
Resolution Done
Additional Detail from JIRA
Votes 0
Component/s Standard Library
Labels Bug
Assignee None
Priority Medium

md5: e993d878672cad630fee8e5c33c63d03

relates to:

  • SR-10709 Swift needs allocation counter tests

Issue Description:

ManagedBufferPointer's docs suggest to use it this way:

class MyBuffer<Element> { // non-@objc
    typealias Manager = ManagedBufferPointer<(Int, Int, String), Element>
    deinit {
        Manager(unsafeBufferObject: self).withUnsafeMutablePointers {
            (pointerToHeader, pointerToElements) -> Void in
                pointerToElements.deinitialize(count: self.count)
                pointerToHeader.deinitialize(count: 1)
    }
}

From this, I created a simple demo (not super useful but...)

class MyBuffer<Element> { // non-@objc
    typealias Manager = ManagedBufferPointer<(Int, Int, String), Element>
    deinit {
        Manager(unsafeBufferObject: self).withUnsafeMutablePointers {
            (pointerToHeader, pointerToElements) -> Void in
                pointerToElements.deinitialize(count: self.count)
                pointerToHeader.deinitialize(count: 1)
    }
}

// All properties are *computed* based on members of the Header
    var count: Int {
        return Manager(unsafeBufferObject: self).header.0
    }
    var capacity: Int {
        return Manager(unsafeBufferObject: self).header.1
    }
    var name: String {
        return Manager(unsafeBufferObject: self).header.2
    }
    static func make(capacity: Int) -> MyBuffer<Element> {
        return Manager(bufferClass: MyBuffer<Element>.self, minimumCapacity: capacity) { (x, f) in
            return (0, capacity, "hello")
        }.buffer as! MyBuffer<Element>
    }
    private init(x: Int) { fatalError() }
}

for cap in 0..<10 {
    let buffer = MyBuffer<Int>.make(capacity: cap)
    precondition(buffer.name == "hello")
    precondition(buffer.count == 0, "\(buffer.count) != 0")
    precondition(buffer.capacity == cap, "\(buffer.capacity) != \(cap)")
}

The expectation (the whole point of ManagedBufferPointer really) would be that this allocates only once per use of MyBuffer. But looking at dtrace:

$ swiftc -O test.swift && sudo dtrace -n 'pid$target::malloc:entry { printf("%s(%d)", probefunc, arg0); }' -c ./test 
dtrace: description 'pid$target::malloc:entry ' matched 4 probes
dtrace: pid 65153 has exited
CPU     ID                    FUNCTION:NAME
  4  12562                     malloc:entry malloc(112)
  4  12562                     malloc:entry malloc(23)
  4  12562                     malloc:entry malloc(48)  ### JW: capacity: 0 (expected)
  4  12562                     malloc:entry malloc(112)
  4  12562                     malloc:entry malloc(32)  ### JW: capacity: 0 (UNEXPECTED)
  4  12562                     malloc:entry malloc(56) ### JW: capacity: 1 (expected)
  4  12562                     malloc:entry malloc(32) ### JW: capacity: 1 (UNEXPECTED)
  4  12562                     malloc:entry malloc(64) ### JW: capacity: 2 (expected)
  4  12562                     malloc:entry malloc(32) ### JW: capacity: 2 (UNEXPECTED)
  4  12562                     malloc:entry malloc(72)
  4  12562                     malloc:entry malloc(32)
  4  12562                     malloc:entry malloc(80)
  4  12562                     malloc:entry malloc(32)
  4  12562                     malloc:entry malloc(88)
  4  12562                     malloc:entry malloc(32)
  4  12562                     malloc:entry malloc(96)
  4  12562                     malloc:entry malloc(32)
  4  12562                     malloc:entry malloc(104)
  4  12562                     malloc:entry malloc(32)
  4  12562                     malloc:entry malloc(112)
  4  12562                     malloc:entry malloc(32)
  4  12562                     malloc:entry malloc(120) ### JW: capacity: 9 (expected)
  4  12562                     malloc:entry malloc(32) ### JW: capacity: 9 (UNEXPECTED)

we can see a pattern here: There's 2 allocations per allocation of MyBuffer which defeats the purpose of using ManagedBufferPointer altogether. We can also use dtrace to figure out where exactly those allocs are coming from:

$ swiftc -O test.swift && sudo dtrace -n 'pid$target::malloc:entry { printf("%s(%d)", probefunc, arg0); ustack(); }' -c ./test 
  8  20662                     malloc:entry malloc(104)
              libsystem_malloc.dylib`malloc
              libswiftCore.dylib`swift_slowAlloc+0x19
              libswiftCore.dylib`_swift_allocObject_(swift::TargetHeapMetadata<swift::InProcess> const*, unsigned long, unsigned long)+0x14
              test`main+0x74
              libdyld.dylib`start+0x1
              test`0x1

  8  20662                     malloc:entry malloc(32)
              libsystem_malloc.dylib`malloc
              libswiftCore.dylib`ManagedBufferPointer.header.read+0x2b
              test`MyBuffer.count.getter+0x80
              test`partial apply for closure #&#8203;1 in MyBuffer.deinit+0x17
              test`partial apply for thunk for @callee_guaranteed (@unowned UnsafeMutablePointer<(Int, Int, String)>, @unowned UnsafeMutablePointer<A>) -> (@error @owned Error)+0x11
              libswiftCore.dylib`ManagedBufferPointer.withUnsafeMutablePointers<A>(_:)+0x49
              test`MyBuffer.deinit+0xb4
              test`MyBuffer.__deallocating_deinit+0x9
              libswiftCore.dylib`_swift_release_dealloc+0x10
              test`main+0x1a8
              libdyld.dylib`start+0x1
              test`0x1

  8  20662                     malloc:entry malloc(112)
              libsystem_malloc.dylib`malloc
              libswiftCore.dylib`swift_slowAlloc+0x19
              libswiftCore.dylib`_swift_allocObject_(swift::TargetHeapMetadata<swift::InProcess> const*, unsigned long, unsigned long)+0x14
              test`main+0x74
              libdyld.dylib`start+0x1
              test`0x1

  8  20662                     malloc:entry malloc(32)
              libsystem_malloc.dylib`malloc
              libswiftCore.dylib`ManagedBufferPointer.header.read+0x2b
              test`MyBuffer.count.getter+0x80
              test`partial apply for closure #&#8203;1 in MyBuffer.deinit+0x17
              test`partial apply for thunk for @callee_guaranteed (@unowned UnsafeMutablePointer<(Int, Int, String)>, @unowned UnsafeMutablePointer<A>) -> (@error @owned Error)+0x11
              libswiftCore.dylib`ManagedBufferPointer.withUnsafeMutablePointers<A>(_:)+0x49
              test`MyBuffer.deinit+0xb4
              test`MyBuffer.__deallocating_deinit+0x9
              libswiftCore.dylib`_swift_release_dealloc+0x10
              test`main+0x1a8
              libdyld.dylib`start+0x1
              test`0x1

  8  20662                     malloc:entry malloc(120)
              libsystem_malloc.dylib`malloc
              libswiftCore.dylib`swift_slowAlloc+0x19
              libswiftCore.dylib`_swift_allocObject_(swift::TargetHeapMetadata<swift::InProcess> const*, unsigned long, unsigned long)+0x14
              test`main+0x74
              libdyld.dylib`start+0x1
              test`0x1

  8  20662                     malloc:entry malloc(32)
              libsystem_malloc.dylib`malloc
              libswiftCore.dylib`ManagedBufferPointer.header.read+0x2b
              test`MyBuffer.count.getter+0x80
              test`partial apply for closure #&#8203;1 in MyBuffer.deinit+0x17
              test`partial apply for thunk for @callee_guaranteed (@unowned UnsafeMutablePointer<(Int, Int, String)>, @unowned UnsafeMutablePointer<A>) -> (@error @owned Error)+0x11
              libswiftCore.dylib`ManagedBufferPointer.withUnsafeMutablePointers<A>(_:)+0x49
              test`MyBuffer.deinit+0xb4
              test`MyBuffer.__deallocating_deinit+0x9
              libswiftCore.dylib`_swift_release_dealloc+0x10
              test`main+0x1a8
              libdyld.dylib`start+0x1
              test`0x1

The expected allocations come from (as expected):

  8  20662                     malloc:entry malloc(120)
              libsystem_malloc.dylib`malloc
              libswiftCore.dylib`swift_slowAlloc+0x19
              libswiftCore.dylib`_swift_allocObject_(swift::TargetHeapMetadata<swift::InProcess> const*, unsigned long, unsigned long)+0x14
              test`main+0x74
              libdyld.dylib`start+0x1
              test`0x1

the unexpected ones seem to come from MyBuffer's deinit:

  8  20662                     malloc:entry malloc(32)
              libsystem_malloc.dylib`malloc
              libswiftCore.dylib`ManagedBufferPointer.header.read+0x2b
              test`MyBuffer.count.getter+0x80
              test`partial apply for closure #&#8203;1 in MyBuffer.deinit+0x17
              test`partial apply for thunk for @callee_guaranteed (@unowned UnsafeMutablePointer<(Int, Int, String)>, @unowned UnsafeMutablePointer<A>) -> (@error @owned Error)+0x11
              libswiftCore.dylib`ManagedBufferPointer.withUnsafeMutablePointers<A>(_:)+0x49
              test`MyBuffer.deinit+0xb4
              test`MyBuffer.__deallocating_deinit+0x9
              libswiftCore.dylib`_swift_release_dealloc+0x10
              test`main+0x1a8
              libdyld.dylib`start+0x1
              test`0x1

This looks like a coroutine allocation?
@weissi
Copy link
Member Author

weissi commented Jul 30, 2019

CC @rjmccall

@belkadan
Copy link
Contributor

cc @Catfish-Man

@lorentey
Copy link
Member

@swift-ci create

@rjmccall
Copy link
Member

rjmccall commented Aug 1, 2019

That does look like a coroutine allocation. It should be temporary. I don't know why this accessor would need to allocate; probably it doesn't and avoiding the allocation would be easy.

@eeckstein
Copy link
Member

Fixed in #27088

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. standard library Area: Standard library umbrella
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@lorentey @weissi @belkadan @rjmccall @eeckstein