Optimise FlatDictionary init for scenarios where no values are stored and llow inlining access#456
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 |
adam-fowler
left a comment
adam-fowler
left a comment
There was a problem hiding this comment.
Looks good, some minor change requests
|
|
||
| @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?
| 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
|
@Joannis can I close this? |
2 participants
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.