Two questions about pointers and performance

Filed SR-12289.

2 Likes

In response to both Question 1 and Question 2: Swift really needs an API for this:
Add withUnsafeElements(to: variable) where variable is any homogenous aggregate

Now for pedantry on Question 2:

Alternative 3 is the intended way to interface with UnsafeRawPointer:

    $0.load(fromByteOffset: Int(i) &* 4, as: UInt32.self)

A withUnsafeElements() API would be nicer because you wouldn't need to do any pointer arithmetic.

The memory binding APIs are not meant for common use, only for writing low-level Swift data types.

    // Alternative 1:
    $0.bindMemory(to: UInt32.self)[Int(i)]

In general, it doesn't make sense to call bindMemory on the address of a local variable, because the variable itself is bound to a type. Rebinding the same memory to a different type implicitly invalidates that variable. In this special case it's correct because UInt32 type is "related" to a homogenous tuple of UInt32 and they are layout compatible. In other words, this call to bindMemory doesn't really have any effect, so it's a safe way to just get a new pointer type. If you we're rebinding memory to Int32 instead, and reading from the results variable again after doing that, then it would be undefined. That's an easy mistake to make, so it's better to just avoid this API altogether unless you're writing a custom memory allocator in Swift!

    // Alternative 2:
    $0.baseAddress!.assumingMemoryBound(to: UInt32.self)[Int(i)]

Calling assumingMemoryBound is only intended to recover from pointer type erasure. Not for casting a pointer to a different type. Again, in this case it's safe because the new UInt32 pointer type is related to and layout compatible with a homogeneous tuple of UInt32. If you "assume" the memory is bound to Int32 instead of UInt32 that would be undefined behavior as soon as you access the pointer. So it's a dangerous abuse of the API, but I think it makes a bit more sense than calling bindMemory.

@Lantua's answers are correct in this case, but layout compatibility is not enough. To avoid undefined behavior, aliasing pointer types also need to have a related type, which basically means one type needs to be a member of the other type. Swift doesn't have any special-case rules about related primitive types. Signed/unsigned types are unrelated.

9 Likes

To elide the copy of the tuple, see if this works for you:

var results = (UInt32(0), UInt32(65536), UInt32(81920), UInt32(86016),
  UInt32(87040), UInt32(87296), UInt32(87360))

public func globalCellIndexB(forGridIndex i: UInt8) -> UInt32 {
  return withUnsafeBytes(of: &results) {
    $0.load(fromByteOffset: Int(i) &* 4, as: UInt32.self)
  }
}

It's weird that we can't automatically do this for constants declared as 'let's, so the bug is good anyway.

1 Like

That unfortunately results in a call to swift_beginAccess, which is worse =/

But if you mark results private, you can get pretty decent codegen.

Yeah, I was assuming the access check would be optimized away. But you probably need either -wmo or private

1 Like

What about allocate(capacity:) on typed variation? Is it already bound, or should I bind it? I'll assume the Raw pointer version is unbound.

Could you elaborate on this part? I don't quite get what makes two types related.

In hind sight, I now notice that in bindMemory(to:) documentation, the warning also mentioned accessing (not binding?) unrelated type.

@Andrew_Trick The whole topic of memory binding would be really fantastic to explain using the compiler’s new “educational notes” feature (swift/userdocs/diagnostics at main · apple/swift · GitHub). Hardly any swift programmers understand what this is about, or what kinds of things are allowed or not allowed. You’ve written some helpful explanations in the past, but they’re scattered across disparate forum threads and github comments; I wonder if you or somebody else wouldn’t mind collecting them in to a single document.

Otherwise (or additionally), it would be a really good topic for a WWDC talk.

6 Likes

Unofficially (the compiler minimally respects this):

Two types are related if any of these conditions hold:

- the types may be identical or aliases of each other

- one type may be a tuple, enum, or struct that contains the other type as part of its own storage

- one type may be an existential such that conforming types may contain the other type as part of its own storage

- both types may be classes and one may be a superclass of the other

Here's my cheat sheet:

6 Likes

@Karl The educational notes feature is awesome.

My feeling about presenting to developers is that Unsafe APIs are an unfortunate necessity but not something we need to promote. When we have a new language feature that makes it unnecessary to use Unsafe APIs, that's clearly worth promoting. On the other hand, a helpful how-to for C interop would be worth presenting and would need to cover UnsafePointer APIs. I'm not deeply involved with that right now, but it would be nice to tackle eventually in conjuction with improvements to the compiler, standard library, and possibly other packages.

I started writing docs on type conversion using UnsafePointer APIs last year, ran out of time, and rescheduled the work for later this year. I'd love to make these educational notes as soon as I get back to it.

The rules for related types are a little tricky. They're a working contract for the compiler but aren't officially part of the language. I suspect we could mention the things we know can't be changed with the caveat that it doesn't constitute an official language spec.

In additional to the docs there are several obvious usability bugs that need to be fixed. It's hard to make good recommendations for best practice without fixing them:

  • [SR-10246] Support limited implicit pointer conversion when calling C functions.
  • [SR-4649] Don't require & to pass value to UnsafeRawPointer
  • [SR-10273] Add an UnsafeRaw[Buffer]Pointer API for loading and storing unaligned/packed data
  • [SR-11082] Support withMemoryRebound(to:) with mismatched sizes
  • Add withUnsafeBufferPointer(to: variable) (no bug yet)
  • [SR-11147] Add withUnsafeElements(to: variable) where variable is any homogenous aggregate
  • [SR-11156] Add UnsafePointer[KeyPath] API that applies the field's offset
  • [SR-11087] Add a closure taking API: UnsafeRaw[Mutable][Buffer]Pointer.withMemoryRebound(to:[capacity:])
  • [SR-11300] Provide a RawPointer version of ManagedBuffer
  • API for "load from contiguous as: T" (no bug yet)
  • Allow comparison of raw and typed pointers (no bug filed)
  • Pointers to uninhabited types, OpaquePointer (no bug?)
  • rdar://25001764 Detect when a temporary pointer to a writeback buffer is persisted after the function returns (no SR yet)
  • rdar://59125380 Custom initializer fails to raise "Inout expression creates a temporary pointer" warning (no SR yet)
7 Likes

@Lantua Right. You never need to explicitly bind memory unless you're writing a memory allocator in Swift or working around some strange C interop problem.

You can see that UnsafeMutablePointer<T>.allocate does it for you in its implementation:

    let rawPtr = Builtin.allocRaw(size._builtinWordValue, align)
    Builtin.bindMemory(rawPtr, count._builtinWordValue, Pointee.self)
    return UnsafeMutablePointer(rawPtr)

And UnsafeMutableRawPointer.initializeMemory also does it for you in its implementation:

    Builtin.bindMemory(_rawValue, count._builtinWordValue, type)
    var nextPtr = self
    for _ in 0..<count {
      Builtin.initialize(repeatedValue, nextPtr._rawValue)
      nextPtr += MemoryLayout<T>.stride
    }
    return UnsafeMutablePointer(_rawValue)
1 Like

It is unfortunate, but it comes up fairly often and is one of the few places where it is possible to fall in to undefined behaviour in Swift. Since safety and lack of undefined behaviour is one of the key features of Swift, it’s still important for developers to know what they’re allowed to do.

The document you posted is exactly the kind of thing that’s been missing. Thanks for that!

4 Likes

I tried your workaround:

private var results = (UInt32(0), UInt32(65536), UInt32(81920), UInt32(86016),
                       UInt32(87040), UInt32(87296), UInt32(87360))

public func globalCellIndexE(forGridIndex i: UInt8) -> UInt32 {
    return withUnsafeBytes(of: &results) {
        $0.load(fromByteOffset: Int(i) &* 4, as: UInt32.self)
    }
}

And it is indeed as fast as the C counterpart.

It doesn't seem to matter if you have private and public there or remove them, at least not when using (-O -whole-module-optimization).

This will also be as fast as the C counterpart:

func globalCellIndexE(forGridIndex i: UInt8) -> UInt32 {
    struct S {
        static var results = (UInt32(0), UInt32(65536), UInt32(81920),
                              UInt32(86016), UInt32(87040), UInt32(87296),
                              UInt32(87360))
    }
    return withUnsafeBytes(of: &S.results) {
        $0.load(fromByteOffset: Int(i) &* 4, as: UInt32.self)
    }
}

Can you please explain why removing the & (from &results or &S.results) makes it more than 5 times slower again?

1 Like

The private/-wmo issue has to do with global variables being slow in Swift because every access is checked for exclusivity... The language feature is worth it for true global variables, and it gets optimized away when the compiler can prove that the variable is never overwritten. A literal should really just be a declared as constant to begin with though...

...but, when the tuple is declared as a 'let', then the compiler does not materialize the value in a stable memory location. The constant value is simply materialized where it's used. That's generally good when you don't need to take the constant's address because the constant will be propagated to its use. The problem is that withUnsafeBytes requires an addressable memory location to give you back an unsafe pointer. So the constant ends up being copied into memory at the point where it's used.

I'm sure it's fixable in the optimizer... so it's good that you filed a bug. Not sure anyone will have time to work on it in the near future.

4 Likes