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

Jump to bottom

[SR-10708] switch over Dictionary lookup retains ref count #53105

Open
drexin opened this issue May 17, 2019 · 3 comments
Open

[SR-10708] switch over Dictionary lookup retains ref count #53105

drexin opened this issue May 17, 2019 · 3 comments
Labels
ARC Feature: automatic reference counting bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself regression SILOptimizer Area → compiler: SIL optimization passes standard library Area: Standard library umbrella swift 5.0

Comments

@drexin
Copy link
Member

drexin commented May 17, 2019

Previous ID SR-10708
Radar None
Original Reporter @drexin
Type Bug

Attachment: Download

Environment

Apple Swift version 5.0 (swift-5.0-RELEASE)
Target: x86_64-apple-darwin18.5.0

Additional Detail from JIRA
Votes 1
Component/s Compiler
Labels Bug, 5.0Regression, ARC, Optimizer
Assignee None
Priority Medium

md5: c994e96b09804105727b513a51ebc355

relates to:

  • SR-10709 Swift needs allocation counter tests

Issue Description:

When switching over the result of a dictionary lookup directly in Swift 5 and then modifying that dictionary it will be copied.

class Foo {
    var dict: [String: Int] = ["x": 1]


    func test(_ x: String) {
        switch dict[x] {
        case .some(let y):
            dict[x] = y
        default: ()
        }
    }
}


let foo = Foo()

for _ in 1 ... 1000 {
    foo.test("x")
}

The above code allocates 1092 times in total.

When assigning the result to a variable first, it does not copy.

class Foo {
    var dict: [String: Int] = ["x": 1]


    func test(_ x: String) {
        let value = dict[x]
        switch value {
        case .some(let y):
            dict[x] = y
        default: ()
        }
    }
}


let foo = Foo()

for _ in 1 ... 1000 {
    foo.test("x")
}

The above code allocates 91 times in total.

In Swift 4.2 both versions do not copy, so this seems to be a regression in Swift 5.
@weissi
Copy link
Member

weissi commented May 17, 2019

attached screenshots of the assembly and SIL diffs between the the good ({{ let value = dict[x]}}) and bad (switch dict[x]) versions. In both cases we see that the switch appears to retain the whole dictionary the the whole switch and in the good version it doesn't...

@weissi
Copy link
Member

weissi commented May 17, 2019

and it's still and issue in a very recent 5.1 snapshot and also in swift-DEVELOPMENT-SNAPSHOT-2019-05-12-a.xctoolchain.

@belkadan
Copy link
Contributor

cc @gottesmm (possibly "fixed" by your redoing of SILGenPattern?)

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@AnthonyLatsis AnthonyLatsis added standard library Area: Standard library umbrella SILOptimizer Area → compiler: SIL optimization passes and removed Optimizer labels Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ARC Feature: automatic reference counting bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself regression SILOptimizer Area → compiler: SIL optimization passes standard library Area: Standard library umbrella swift 5.0
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@drexin @weissi @belkadan @AnthonyLatsis