Merge pull request #2528 from apple/stdlib-dictionary-refactor

[WIP, don't merge] stdlib: refactor unsafe parts of Dictionary

Author: gribozavr

stdlib/public/core/Builtin.swift

@@ -61,17 +61,45 @@ public func strideofValue<T>(_:T) -> Int {
   return strideof(T.self)
 }
 
+// This function is the implementation of the `_roundUp` overload set.  It is
+// marked `@inline(__always)` to make primary `_roundUp` entry points seem
+// cheap enough for the inliner.
 @_versioned
-internal func _roundUp(_ offset: Int, toAlignment alignment: Int) -> Int {
-  _sanityCheck(offset >= 0)
+@inline(__always)
+internal func _roundUpImpl(_ offset: UInt, toAlignment alignment: Int) -> UInt {
   _sanityCheck(alignment > 0)
   _sanityCheck(_isPowerOf2(alignment))
   // Note, given that offset is >= 0, and alignment > 0, we don't
   // need to underflow check the -1, as it can never underflow.
-  let x = (offset + alignment &- 1)
+  let x = offset + UInt(bitPattern: alignment) &- 1
   // Note, as alignment is a power of 2, we'll use masking to efficiently
   // get the aligned value
-  return x & ~(alignment &- 1)
+  return x & ~(UInt(bitPattern: alignment) &- 1)
+}
+
+@_versioned
+internal func _roundUp(_ offset: UInt, toAlignment alignment: Int) -> UInt {
+  return _roundUpImpl(offset, toAlignment: alignment)
+}
+
+@_versioned
+internal func _roundUp(_ offset: Int, toAlignment alignment: Int) -> Int {
+  _sanityCheck(offset >= 0)
+  return Int(_roundUpImpl(UInt(bitPattern: offset), toAlignment: alignment))
+}
+
+@_versioned
+internal func _roundUp<T, DestinationType>(
+  _ pointer: UnsafeMutablePointer<T>,
+  toAlignmentOf destinationType: DestinationType.Type
+) -> UnsafeMutablePointer<DestinationType> {
+  // Note: unsafe unwrap is safe because this operation can only increase the
+  // value, and can not produce a null pointer.
+  return UnsafeMutablePointer<DestinationType>(
+    bitPattern: _roundUpImpl(
+      UInt(bitPattern: pointer),
+      toAlignment: alignof(DestinationType))
+  ).unsafelyUnwrapped
 }
 
 /// Returns a tri-state of 0 = no, 1 = yes, 2 = maybe.

stdlib/public/core/CMakeLists.txt

@@ -114,6 +114,7 @@ set(SWIFTLIB_ESSENTIAL
   UnicodeScalar.swift
   UnicodeTrie.swift.gyb
   Unmanaged.swift
+  UnsafeBitMap.swift
   UnsafeBufferPointer.swift.gyb
   UnsafePointer.swift.gyb
   WriteBackMutableSlice.swift

stdlib/public/core/FixedPoint.swift.gyb

@@ -701,6 +701,14 @@ internal func _unsafeMinus(_ lhs: Int, _ rhs: Int) -> Int {
 #endif
 }
 
+internal func _unsafeMultiply(_ lhs: Int, _ rhs: Int) -> Int {
+#if INTERNAL_CHECKS_ENABLED
+  return lhs * rhs
+#else
+  return lhs &* rhs
+#endif
+}
+
 @available(*, unavailable, renamed: "Integer")
 public typealias IntegerType = Integer
 

stdlib/public/core/GroupInfo.json

@@ -140,5 +140,6 @@
     "Process.swift",
     "Tuple.swift",
     "NewtypeWrapper.swift",
+    "UnsafeBitMap.swift"
   ]
 }

stdlib/public/core/HashedCollections.swift.gyb

@@ -2455,64 +2455,6 @@ collections = [
 ]
 }%
 
-/// A wrapper around a bitmap storage with room for at least `bitCount` bits.
-internal struct _BitMap {
-  internal let values: UnsafeMutablePointer<UInt>
-  internal let bitCount: Int
-
-  // Note: We use UInt here to get unsigned math (shifts).
-  internal static func wordIndex(_ i: UInt) -> UInt {
-    return i / UInt._sizeInBits
-  }
-
-  internal static func bitIndex(_ i: UInt) -> UInt {
-    return i % UInt._sizeInBits
-  }
-
-  internal static func wordsFor(_ bitCount: Int) -> Int {
-    return bitCount + Int._sizeInBytes - 1 / Int._sizeInBytes
-  }
-
-  internal init(storage: UnsafeMutablePointer<UInt>, bitCount: Int) {
-    self.bitCount = bitCount
-    self.values = storage
-  }
-
-  internal var numberOfWords: Int {
-    get {
-      return _BitMap.wordsFor(bitCount)
-    }
-  }
-
-  internal func initializeToZero() {
-    for i in 0 ..< numberOfWords {
-      (values + i).initialize(with: 0)
-    }
-  }
-
-  internal subscript(i: Int) -> Bool {
-    get {
-      _sanityCheck(i < Int(bitCount) && i >= 0, "index out of bounds")
-      let idx = UInt(i)
-      let word = values[Int(_BitMap.wordIndex(idx))]
-      let bit = word & (1 << _BitMap.bitIndex(idx))
-      return bit != 0
-    }
-    nonmutating set {
-      _sanityCheck(i < Int(bitCount) && i >= 0, "index out of bounds")
-      let idx = UInt(i)
-      let wordIdx = _BitMap.wordIndex(idx)
-      if newValue {
-        values[Int(wordIdx)] =
-            values[Int(wordIdx)] | (1 << _BitMap.bitIndex(idx))
-      } else {
-        values[Int(wordIdx)] =
-            values[Int(wordIdx)] & ~(1 << _BitMap.bitIndex(idx))
-      }
-    }
-  }
-}
-
 /// Header part of the native storage.
 internal struct _HashedContainerStorageHeader {
   internal init(capacity: Int) {
@@ -2548,8 +2490,8 @@ final internal class _Native${Self}StorageImpl<${TypeParameters}> :
   /// Returns the bytes necessary to store a bit map of 'capacity' bytes and
   /// padding to align the start to word alignment.
   internal static func bytesForBitMap(capacity: Int) -> Int {
-    let numWords = _BitMap.wordsFor(capacity)
-    return numWords * sizeof(UInt) + alignof(UInt)
+    let numWords = _UnsafeBitMap.sizeInWords(forSizeInBits: capacity)
+    return numWords * strideof(UInt) + alignof(UInt)
   }
 
   /// Returns the bytes necessary to store 'capacity' keys and padding to align
@@ -2560,12 +2502,11 @@ final internal class _Native${Self}StorageImpl<${TypeParameters}> :
     return strideof(Key.self) * capacity + padding
   }
 
+%if Self == 'Dictionary':
   /// Returns the bytes necessary to store 'capacity' values and padding to
   /// align the start to the alignment of the 'Value' type assuming a base
   /// address aligned to the maximum of the alignment of the 'Key' type and the
   /// alignment of a word.
-
-%if Self == 'Dictionary':
   internal static func bytesForValues(capacity: Int) -> Int {
     let maxPrevAlignment = max(alignof(Key.self), alignof(UInt))
     let padding = max(0, alignof(Value.self) - maxPrevAlignment)
@@ -2574,9 +2515,7 @@ final internal class _Native${Self}StorageImpl<${TypeParameters}> :
 %end
 
   internal var buffer: BufferPointer {
-    get {
-      return BufferPointer(self)
-    }
+    return BufferPointer(self)
   }
 
   // All underscored functions are unsafe and need a _fixLifetime in the caller.
@@ -2591,9 +2530,7 @@ final internal class _Native${Self}StorageImpl<${TypeParameters}> :
 
   @_versioned
   internal var _capacity: Int {
-    get {
-      return _body.capacity
-    }
+    return _body.capacity
   }
 
   @_versioned
@@ -2607,45 +2544,30 @@ final internal class _Native${Self}StorageImpl<${TypeParameters}> :
   }
 
   internal var _maxLoadFactorInverse : Double {
-    get {
-      return _body.maxLoadFactorInverse
-    }
+    return _body.maxLoadFactorInverse
   }
 
   internal
   var _initializedHashtableEntriesBitMapStorage: UnsafeMutablePointer<UInt> {
-    get {
-      let start = UInt(Builtin.ptrtoint_Word(buffer._elementPointer._rawValue))
-      let alignment = UInt(alignof(UInt))
-      let alignMask = alignment &- UInt(1)
-      return UnsafeMutablePointer<UInt>(
-          bitPattern:(start &+ alignMask) & ~alignMask)!
-    }
+    return _roundUp(buffer._elementPointer, toAlignmentOf: UInt.self)
   }
 
   internal var _keys: UnsafeMutablePointer<Key> {
-    get {
-      let start =
-          UInt(Builtin.ptrtoint_Word(
-              _initializedHashtableEntriesBitMapStorage._rawValue)) &+
-          UInt(_BitMap.wordsFor(_capacity)) &* UInt(strideof(UInt))
-      let alignment = UInt(alignof(Key))
-      let alignMask = alignment &- UInt(1)
-      return UnsafeMutablePointer<Key>(
-          bitPattern:(start &+ alignMask) & ~alignMask)!
-    }
+    let bitMapSizeInBytes =
+      _unsafeMultiply(
+        _UnsafeBitMap.sizeInWords(forSizeInBits: _capacity),
+        strideof(UInt))
+    let start =
+      UnsafeMutablePointer<UInt8>(_initializedHashtableEntriesBitMapStorage)
+      + bitMapSizeInBytes
+    return _roundUp(start, toAlignmentOf: Key.self)
   }
 
 %if Self == 'Dictionary':
   internal var _values: UnsafeMutablePointer<Value> {
-    get {
-      let start = UInt(Builtin.ptrtoint_Word(_keys._rawValue)) &+
-        UInt(_capacity) &* UInt(strideof(Key.self))
-      let alignment = UInt(alignof(Value))
-      let alignMask = alignment &- UInt(1)
-      return UnsafeMutablePointer<Value>(
-          bitPattern:(start &+ alignMask) & ~alignMask)!
-    }
+    let keysSizeInBytes = _unsafeMultiply(_capacity, strideof(Key.self))
+    let start = UnsafeMutablePointer<UInt8>(_keys) + keysSizeInBytes
+    return _roundUp(start, toAlignmentOf: Value.self)
   }
 %end
 
@@ -2655,14 +2577,14 @@ final internal class _Native${Self}StorageImpl<${TypeParameters}> :
     let requiredCapacity =
       bytesForBitMap(capacity: capacity) + bytesForKeys(capacity: capacity)
 %if Self == 'Dictionary':
-        + bytesForValues(capacity: capacity)
+      + bytesForValues(capacity: capacity)
 %end
 
     let r = super.create(minimumCapacity: requiredCapacity) { _ in
       return _HashedContainerStorageHeader(capacity: capacity)
     }
     let storage = r as! StorageImpl
-    let initializedEntries = _BitMap(
+    let initializedEntries = _UnsafeBitMap(
         storage: storage._initializedHashtableEntriesBitMapStorage,
         bitCount: capacity)
     initializedEntries.initializeToZero()
@@ -2671,7 +2593,7 @@ final internal class _Native${Self}StorageImpl<${TypeParameters}> :
 
   deinit {
     let capacity = _capacity
-    let initializedEntries = _BitMap(
+    let initializedEntries = _UnsafeBitMap(
         storage: _initializedHashtableEntriesBitMapStorage, bitCount: capacity)
     let keys = _keys
 %if Self == 'Dictionary':
@@ -2721,17 +2643,17 @@ struct _Native${Self}Storage<${TypeParametersDecl}> :
 
   internal let buffer: StorageImpl
 
-  internal let initializedEntries: _BitMap
+  internal let initializedEntries: _UnsafeBitMap
   internal let keys: UnsafeMutablePointer<Key>
 %if Self == 'Dictionary':
   internal let values: UnsafeMutablePointer<Value>
 %end
 
   internal init(capacity: Int) {
     buffer = StorageImpl.create(capacity: capacity)
-    initializedEntries = _BitMap(
-        storage: buffer._initializedHashtableEntriesBitMapStorage,
-        bitCount: capacity)
+    initializedEntries = _UnsafeBitMap(
+      storage: buffer._initializedHashtableEntriesBitMapStorage,
+      bitCount: capacity)
     keys = buffer._keys
 %if Self == 'Dictionary':
     values = buffer._values
@@ -2754,11 +2676,9 @@ struct _Native${Self}Storage<${TypeParametersDecl}> :
   @_transparent
   public // @testable
   var capacity: Int {
-    get {
-      let result = buffer._capacity
-      _fixLifetime(buffer)
-      return result
-    }
+    let result = buffer._capacity
+    _fixLifetime(buffer)
+    return result
   }
 
   @_versioned
@@ -2777,11 +2697,9 @@ struct _Native${Self}Storage<${TypeParametersDecl}> :
 
   @_transparent
   internal var maxLoadFactorInverse: Double {
-    get {
-      let result = buffer._maxLoadFactorInverse
-      _fixLifetime(buffer)
-      return result
-    }
+    let result = buffer._maxLoadFactorInverse
+    _fixLifetime(buffer)
+    return result
   }
 
   @_versioned
@@ -3139,16 +3057,15 @@ internal struct _BridgedNative${Self}Storage {
   internal typealias SequenceElement = ${AnySequenceType}
 
   internal let buffer: StorageImpl
-  internal let initializedEntries: _BitMap
+  internal let initializedEntries: _UnsafeBitMap
   internal let keys: UnsafeMutablePointer<AnyObject>
 %if Self == 'Dictionary':
   internal let values: UnsafeMutablePointer<AnyObject>
 %end
 
-
   internal init(buffer: StorageImpl) {
     self.buffer = buffer
-    initializedEntries = _BitMap(
+    initializedEntries = _UnsafeBitMap(
       storage: buffer._initializedHashtableEntriesBitMapStorage,
       bitCount: buffer._capacity)
     keys = buffer._keys
@@ -3160,11 +3077,9 @@ internal struct _BridgedNative${Self}Storage {
 
   @_transparent
   internal var capacity: Int {
-    get {
-      let result = buffer._capacity
-      _fixLifetime(buffer)
-      return result
-    }
+    let result = buffer._capacity
+    _fixLifetime(buffer)
+    return result
   }
 
   @_versioned