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-11311] Class property accessed as rvalue wrongly forces copy-on-write. #53712

Open
atrick opened this issue Aug 15, 2019 · 3 comments
Open
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself

Comments

@atrick
Copy link
Member

atrick commented Aug 15, 2019

Previous ID SR-11311
Radar rdar://problem/54335055
Original Reporter @atrick
Type Bug
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug
Assignee None
Priority Medium

md5: 3201009b86536a426ec1c39520abafba

Issue Description:

When accessing a class property as an rvalue, the value is retained beyond where it is needed to materialize the rvalue. So, when the left side of an assignment is an array subscript modify access and the right side is an array subscript getter, the entire array is copied.

This must be a known issue, so I suspect there are other bugs, but I figure it's worth filing one specific to the newly added SortArrayInClass benchmark:
#26663

Reduced test case:

public class ArrayWrapper {
  private var array: [Int]

  public func copyElement(from: Int, to: Int) {
    array[to] = array[from]
  }

  init(array: [Int]) { self.array = array }
}

Note that copyElement will always copy the entire array:

  // load the array property and retain it
  %6 = ref_element_addr %2 : $ArrayWrapper, #ArrayWrapper.array
  %7 = begin_access [read] [dynamic] %6 : $*Array<Int>
  %8 = load %7 : $*Array<Int>
  retain_value %8 : $Array<Int>
  end_access %7 : $*Array<Int>

  // load the from element, no problem here
  %11 = alloc_stack $Int
  // function_ref Array.subscript.getter
  %12 = function_ref @$sSayxSicig : $@convention(method) <τ_0_0> (Int, @guaranteed Array<τ_0_0>) -> @out τ_0_0
  %13 = apply %12<Int>(%11, %0, %8) : $@convention(method) <τ_0_0> (Int, @guaranteed Array<τ_0_0>) -> @out τ_0_0
  %14 = load %11 : $*Int

  // modify access while the array is still retained forces a copy
  %15 = ref_element_addr %2 : $ArrayWrapper, #ArrayWrapper.array
  %16 = begin_access [modify] [dynamic] %15 : $*Array<Int>
  // function_ref Array.subscript.modify
  %17 = function_ref @$sSayxSiciM : $@yield_once @convention(method) <τ_0_0> (Int, @inout Array<τ_0_0>) -> @yields @inout τ_0_0
  (%18, %19) = begin_apply %17<Int>(%1, %16) : $@yield_once @convention(method) <τ_0_0> (Int, @inout Array<τ_0_0>) -> @yields @inout τ_0_0
  store %14 to %18 : $*Int
  end_apply %19
  end_access %16 : $*Array<Int>

  // Why wasn't the array released before the modify? This array value
  // is only used in the right side of the assignment!
  release_value %8 : $Array<Int>
@atrick
Copy link
Member Author

atrick commented Aug 15, 2019

@swift-cicreate

@atrick
Copy link
Member Author

atrick commented Aug 15, 2019

@compnerd here's a bug that we can use to track the quicksort performance problem... where it takes 1500x times to long to sort.

@atrick
Copy link
Member Author

atrick commented Aug 15, 2019

@rjmccall aschwaighofer@apple.com (JIRA User) I don't remember at the moment whether we were planning to optimize this case, whether it's a known issue or regression. I'm filing this to keep track of it since I don't have time to investigate further now.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 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. compiler The Swift compiler in itself
Projects
None yet
Development

No branches or pull requests

1 participant