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-13438] Many uses of existentials should optimize much better. #55879

Open
dabrahams opened this issue Aug 24, 2020 · 4 comments
Open

[SR-13438] Many uses of existentials should optimize much better. #55879

dabrahams opened this issue Aug 24, 2020 · 4 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself

Comments

@dabrahams
Copy link
Collaborator

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

md5: 19fe883fffc5d9d7a6619f88d2dbb8a1

Issue Description:

Take for example:

func disassembleMe0(x: Any) -> Bool {
  x is Int
}

which generates the following x86:

x.disassembleMe0(x: Any) -> Swift.Bool:
    pushq   %rbp
    movq    %rsp, %rbp
    pushq   %rbx
    subq    $40, %rsp
    leaq    -48(%rbp), %rbx
    movq    %rbx, %rsi
    callq   outlined init with copy of Any
    movq    type metadata for Any@GOTPCREL(%rip), %rdx
    addq    $8, %rdx
    movq    type metadata for Swift.Int@GOTPCREL(%rip), %rcx
    leaq    -16(%rbp), %rdi
    movl    $6, %r8d
    movq    %rbx, %rsi
    callq   _swift_dynamicCast
    addq    $40, %rsp
    popq    %rbx
    popq    %rbp
    retq

It should simply be checking the type metadata pointer in the any to see if it matches `Int`'s.

Another example:

func disassembleMe1(x: Any) -> Int {
  (x as? Int).unsafelyUnwrapped
}

which generates:

x.disassembleMe1(x: Any) -> Swift.Int:
    pushq   %rbp
    movq    %rsp, %rbp
    pushq   %rbx
    subq    $40, %rsp
    leaq    -48(%rbp), %rbx
    movq    %rbx, %rsi
    callq   outlined init with copy of Any
    movq    type metadata for Any@GOTPCREL(%rip), %rdx
    addq    $8, %rdx
    movq    type metadata for Swift.Int@GOTPCREL(%rip), %rcx
    leaq    -16(%rbp), %rdi
    movl    $6, %r8d
    movq    %rbx, %rsi
    callq   _swift_dynamicCast
    testb   %al, %al
    je  LBB4_1
    movq    -16(%rbp), %rax
    jmp LBB4_3
LBB4_1:
    xorl    %eax, %eax
LBB4_3:
    addq    $40, %rsp
    popq    %rbx
    popq    %rbp
    retq

but should actually unsafely pull the `Int` from the Any's inline storage.

I've created this gist, which crawls around in Any's storage layout to avoid these overheads, but such hackery should be unnecessary for the majority of cases.

I realize that even after these performance issues are addressed, we'll still need my hackery to enable efficient in-place mutation; features to enable that to be expressed cleanly in the language are topics for a different bug report.

/cc @eeckstein
@dabrahams
Copy link
Collaborator Author

Moreover, even the hacks are incredibly sensitive to the shape of code. For example, just replacing the pointer method in the gist with

  private func pointer<T>(toStored: T.Type) -> UnsafeMutablePointer<T> {
    withUnsafePointer(to: storage) { Handle<T>($0) }.pointer // "storage" was "self"
  }

Produces much worse code for disassembleMe0
as does eliminating Handle and replacing the body of pointer with this implementation, which should be simpler overall:

  /// Returns a pointer to the `T` which is assumed to be stored in `self`.
  private func pointer<T>(toStored: T.Type) -> UnsafeMutablePointer<T> {
    typealias Box = (metadata: UnsafeRawPointer, refCounts: Int, value: T)
    typealias BoxPointer = UnsafeMutablePointer<Box>
    typealias BoxedLayout = (boxAddress: BoxPointer, Int, Int, storedType: Any.Type)
    
    let address = withUnsafePointer(to: storage) { UnsafeMutableRawPointer(mutating: $0) }
      
    if MemoryLayout<(T, Any.Type)>.size <= MemoryLayout<Any>.size
         && MemoryLayout<T>.alignment <= MemoryLayout<Any>.alignment {
      return address.assumingMemoryBound(to: T.self)
    }
    else {
      return withUnsafeMutablePointer(
        to: &address.assumingMemoryBound(to: BoxedLayout.self)[0].boxAddress[0].value
      ) { $0 }
    } 
  }

@typesanitizer
Copy link

@swift-ci create

@dabrahams
Copy link
Collaborator Author

Can't remember whether the sync system handles edits to existing comments, so moving my edit here. Sorry for the radar noise.

as does eliminating Handle and replacing the body of pointer with this implementation, which should be simpler overall:

  /// Returns a pointer to the `T` which is assumed to be stored in `self`.
  private func pointer<T>(toStored: T.Type) -> UnsafeMutablePointer<T> {
    typealias Box = (metadata: UnsafeRawPointer, refCounts: Int, value: T)
    typealias BoxPointer = UnsafeMutablePointer<Box>
    typealias BoxedLayout = (boxAddress: BoxPointer, Int, Int, storedType: Any.Type)
    
    let address = withUnsafePointer(to: storage) { UnsafeMutableRawPointer(mutating: $0) }
      
    if MemoryLayout<(T, Any.Type)>.size <= MemoryLayout<Any>.size
         && MemoryLayout<T>.alignment <= MemoryLayout<Any>.alignment {
      return address.assumingMemoryBound(to: T.self)
    }
    else {
      return withUnsafeMutablePointer(
        to: &address.assumingMemoryBound(to: BoxedLayout.self)[0].boxAddress[0].value
      ) { $0 }
    } 
  }

@tbkka
Copy link
Contributor

tbkka commented Sep 1, 2020

Unfortunately, `Any is Int` cannot be simplified quite this far. In particular, `Any` might contain:

  • An `Optional<Int>`

  • An `NSNumber` reference which can be bridged to `Int`

  • A nested `Any` with one of the above

None of these would be handled correctly by simply comparing the metatype reference from the `Any` to see if it matched `Int.self`.

While we can't reduce this quite as far as you're suggesting, we could certainly improve cases like this:

  • We might add a runtime ABI specifically for Any checks (which are common and quite likely worth optimizing). In particular, this would eliminate looking up the `Any` metatype

  • We could improve constant folding for existentials so that the compiler can handle more casts of this sort at compile time.

@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
Assignees
No one assigned
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@dabrahams @typesanitizer @tbkka