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.

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

[SR-12125] String(decoding:from:) doesn't try withContiguousStorageIfAvailable #54560

Closed
weissi opened this issue Feb 4, 2020 · 4 comments
Closed
Assignees
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. performance standard library Area: Standard library umbrella

Comments

@weissi
Copy link
Member

weissi commented Feb 4, 2020

Previous ID SR-12125
Radar rdar://problem/59148099
Original Reporter @weissi
Type Bug

Attachment: Download

Environment

swift 5.1 and 5.2 branch

Additional Detail from JIRA
Votes 1
Component/s Standard Library
Labels Bug, Performance
Assignee @milseman
Priority Medium

md5: 4785e0cdc39c1c82c168b48e24fef33d

Issue Description:

String(decoding:from🙂 does sometimes make intermediate Array copies of the Collection that it receives. That makes sense because the collection might not be contiguous. If however the collection implements withContiguousStorageIfAvailable, then String(decoding:from🙂 should use this.

The attached program shows this (parts reproduced here):

struct MyCollection: Collection {
    let storage: [UInt8]
    typealias Element = UInt8
    typealias Index = Array<UInt8>.Index
    typealias SubSequence = Array<UInt8>.SubSequence
    typealias Indices = Array<UInt8>.Indices

    init(underlying: [UInt8]) {
        self.storage = underlying
    }

    public var indices: Indices {
        return self.storage.indices
    }
    public subscript(bounds: Range<Index>) -> SubSequence {
        return self.storage[bounds]
    }

    public subscript(position: Index) -> Element {
        return self.storage[position]
    }

    public var startIndex: Index {
        return self.storage.startIndex
    }

    public var endIndex: Index {
        return self.storage.endIndex
    }

    func index(after i: Index) -> Index {
        return self.storage.index(after: i)
    }

    func withContiguousStorageIfAvailable<R>(_ body: (UnsafeBufferPointer<UInt8>) throws -> R) rethrows -> R? {
        return try self.storage.withUnsafeBufferPointer(body)
    }
}

func makeString<Bytes: Collection>(_ bytes: Bytes) -> String where Bytes.Element == UInt8 {
    return String(decoding: bytes, as: Unicode.UTF8.self)
}

func testMyCollectionUnsafe(_ array: MyCollection) -> String {
    return array.withContiguousStorageIfAvailable {
        return makeString($0)
    }!
}

func testMyCollection(_ array: MyCollection) -> String {
    return makeString(array)
}

@inline(never)
func doMyCollectionUnsafe() {
    let array: [UInt8] = Array(repeating: UInt8(ascii: "X"), count: 15)
    let myCollection = MyCollection(underlying: array)

    for _ in 0..<10000 {
        precondition(testMyCollectionUnsafe(myCollection) == "XXXXXXXXXXXXXXX")
    }
}

@inline(never)
func doMyCollection() {
    let array: [UInt8] = Array(repeating: UInt8(ascii: "X"), count: 15)
    let myCollection = MyCollection(underlying: array)

    for _ in 0..<10000 {
        precondition(testMyCollection(myCollection) == "XXXXXXXXXXXXXXX")
    }
}

doMyCollection()
doMyCollectionUnsafe()

If we look at all allocations that happen more than once we see only

              libsystem_malloc.dylib`malloc
              libswiftCore.dylib`swift_slowAlloc+0x19
              libswiftCore.dylib`swift_allocObject+0x27
              test`specialized _ContiguousArrayBuffer.init(_uninitializedCount:minimumCapacity:)+0x39
              test`specialized Collection._copyToContiguousArray()+0x4f
              test`specialized makeString<A>(_:)+0x11a
              test`doMyCollection()+0xb2
              test`main+0x13
              libdyld.dylib`start+0x1
              test`0x1
            10000

so doMyCollection allocates 10,000 times but doMyCollectionUnsafe does not.

repro

swiftc -O test.swift && sudo ~/devel/swift-nio/dev/malloc-aggregation.d -c ./test
@weissi
Copy link
Member Author

weissi commented Feb 4, 2020

@swift-ci create

@weissi
Copy link
Member Author

weissi commented Feb 4, 2020

CC @milseman

@milseman
Copy link
Mannequin

milseman mannequin commented Mar 31, 2020

#30729

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@weissi weissi changed the title [SR-12125] String(decoding:from:) doesn't try `withContiguousStorageIfAvailable [SR-12125] String(decoding:from:) doesn't try withContiguousStorageIfAvailable Apr 25, 2022
@weissi
Copy link
Member Author

weissi commented May 4, 2022

fixed.

@weissi weissi closed this as completed May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. performance standard library Area: Standard library umbrella
Projects
None yet
Development

No branches or pull requests

1 participant