Optimise FlatDictionary init for scenarios where no values are stored and llow inlining access #456
Conversation
…. Allow inlining access, providing specialized access
|
Empty arrays don't allocate any memory, so I'm not sure this is going to be a saving, as you have added a branch whenever accessing the parameters now |
|
I think my measurements are macOS-specific then. Swift arrays do seem to have a different (default) implementation from the ContiguousArray one |
| @@ -26,104 +26,154 @@ public struct FlatDictionary<Key: Hashable, Value>: Collection, ExpressibleByDic | |||
| public typealias Element = (key: Key, value: Value) | |||
| public typealias Index = Array<Element>.Index | |||
|
|
|||
| @usableFromInline | |||
| struct Storage { | |||
| @usableFromInline internal /* private */ var elements: [Element] | |||
There was a problem hiding this comment.
Can we remove the commented out /* private */ if we require everything to be internal
| /// Access element at specific position | ||
| public subscript(_ index: Index) -> Element { return self.elements[index] } | ||
| @inlinable | ||
| public subscript(_ index: Index) -> Element { return self.storage!.elements[index] } |
There was a problem hiding this comment.
! isn't ideal here, perhaps we should add a precondition that storage exist and provide an out of range message or something similar, given the index will be out of range
| /// Returns the index immediately after the given index | ||
| public func index(after index: Index) -> Index { self.elements.index(after: index) } | ||
| @inlinable | ||
| public func index(after index: Index) -> Index { index + 1 } |
There was a problem hiding this comment.
Any reason you changed this? I know index+1 is correct in most cases, but what is the expected result when index == endIndex?
| } else { | ||
| return nil | ||
| } | ||
| } | ||
| set { | ||
| let hashKey = Self.hashKey(key) | ||
| if let index = hashKeys.firstIndex(of: hashKey) { | ||
| if var storage, let index = storage.hashKeys.firstIndex(of: hashKey) { |
There was a problem hiding this comment.
The ordering of the if statements here is slightly confusing. Currently you test for the existence of storage twice when the key doesn't exist. Perhaps it should be
if let storage {
// do stuff when storage already exists
} else {
// create storage if needed
}| } | ||
|
|
||
| // FlatDictionary is Sendable when Key and Value are Sendable | ||
| extension FlatDictionary.Storage: Sendable where Key: Sendable, Value: Sendable {} |
There was a problem hiding this comment.
I added this in to get rid of a Sendable warning
This provides specialized access to the type, and virtually eliminates its presence from routes where a flat dictionary is not used. This affects the route parameters primarily, but helps in particular because
CoreRequestContextalways starts out with an emptyParametersinstance.