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-11675] Unexpected CoW when mutating a struct-wrapped value #54084

Closed
glbrntt opened this issue Oct 28, 2019 · 7 comments
Closed

[SR-11675] Unexpected CoW when mutating a struct-wrapped value #54084

glbrntt opened this issue Oct 28, 2019 · 7 comments
Assignees
@eeckstein
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself performance

Comments

@glbrntt
Copy link

glbrntt commented Oct 28, 2019

Previous ID SR-11675
Radar rdar://problem/56704059
Original Reporter @glbrntt
Type Bug
Status Resolved
Resolution Done

Attachment: Download

Environment

  • Swift 5.1 release (macOS)

  • Swift DEVELOPMENT-SNAPSHOT-2019-10-24

Additional Detail from JIRA
Votes 1
Component/s Compiler
Labels Bug, Performance
Assignee @eeckstein
Priority Medium

md5: 3b7ebb30252bfff9776e6eee5de924f3

Issue Description:

The following code CoWs when modifying data via a wrapper struct:

struct ArrayWrapper {
    var data: [Int]
}

func run() {
    for _ in 0..<10_000 {
        var data = Array<Int>(repeating: 0, count: 1024)
        data[0] = 24  // Does not CoW

        var wrapper = ArrayWrapper(data: data)
        wrapper.data[0] = 42  // CoWs
    }
}

run()

Since data is not used after wrapper "takes ownership" of it, I would expect the compiler to be able to figure out that wrapper's reference to {{data}}s backing storage is unique, and therefore should not incur a CoW when modified.

Output of running NIO's malloc-aggregation.d script:

...

libsystem_malloc.dylib`malloc
libswiftCore.dylib`swift_slowAlloc+0x19
libswiftCore.dylib`_swift_allocObject_(swift::TargetHeapMetadata<swift::InProcess> const*, unsigned long, unsigned long)+0x14
UnwantedAlloc`specialized _ArrayBufferProtocol.init(copying:)+0x51
UnwantedAlloc`run()+0x93
UnwantedAlloc`main+0x9
libdyld.dylib`start+0x1
UnwantedAlloc`0x1
10000


libsystem_malloc.dylib`malloc
libswiftCore.dylib`swift_slowAlloc+0x19
libswiftCore.dylib`_swift_allocObject_(swift::TargetHeapMetadata<swift::InProcess> const*, unsigned long, unsigned long)+0x14
libswiftCore.dylib`_ContiguousArrayBuffer.init(_uninitializedCount:minimumCapacity:)+0x6f
UnwantedAlloc`run()+0x2d
UnwantedAlloc`main+0x9
libdyld.dylib`start+0x1
UnwantedAlloc`0x1
10000
@belkadan
Copy link
Contributor

cc @eeckstein

@eeckstein
Copy link
Member

@swift-ci create

@eeckstein
Copy link
Member

This needs support of the SIL begin_apply instruction in the DestroyHoisting optimization

@eeckstein
Copy link
Member

fixed in #27926

@glbrntt
Copy link
Author

glbrntt commented Nov 5, 2019

Awesome, thanks for the quick resolution @eeckstein! Is there any expectation that this should work for debug builds as well?

@eeckstein
Copy link
Member

Technically it would not be a problem to enable this optimisation in Onone, too. There are two issues with this:

  1. Onone compile time, which I think is not a big issue for this optimization.
  2. debugability: In theory you could modify 'data' in the debugger after creating 'wrapper'. Then 'data' and 'wrapper.data' would be two different arrays. But with this optimization they arrays share the same buffer and you would observe the modification of 'data' also in 'wrapper.data'. I'm not sure how important this is, but it needs to be discussed.

@glbrntt
Copy link
Author

glbrntt commented Nov 6, 2019

Thanks Erik. This isn't something we particularly need, I was just curious.

@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

@eeckstein eeckstein

Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself performance
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@belkadan @glbrntt @eeckstein