[WIP][stdlib] Dictionary: Add unsafe bulk-loading initializer #21208
Conversation
Interface inspired by this Array pitch: https://forums.swift.org/t/array-initializer-with-access-to-uninitialized-buffer/13689
|
cc @Catfish-Man |
|
@swift-ci please test |
| } else { | ||
| // Swap into place, then try again with the swapped-in value. | ||
| native.swapEntry(target, with: bucket) | ||
| } |
There was a problem hiding this comment.
This is a cool algorithm, @lorentey 👏
There was a problem hiding this comment.
I had to work it out myself because I couldn't picture how to hash-in-place like this, but I think I got it. Nice work. (Did you adapt it from somewhere or come up with it yourself?)
Does this obsolete DictionaryBuilder?
For my own understanding:
- The storage is separated into three regions: "before
bucket", "bucketup tocount", and "countand after". - Everything in "before
bucket" and "countand after" is either uninitialized or in use. Everything in "bucketup tocount" is either unprocessed or in use. - "in use" is tracked by the bitmap in
native.hashTable, the same way it would be for a working Dictionary. Whenever we see such a bucket in the middle section, we skip over it, andnative.hashTable.insertNewwill never return an in-use bucket. (This is the key thing I was missing when trying to picture the algorithm on my own.) - When processing an item, we're basically deciding how to move it from the middle section. Either we're moving it elsewhere in the middle section, in which case we do a swap so that we don't lose the unprocessed item that was there, or we're moving it outside the middle section, in which case it's okay to leave behind an uninitialized hole (because we're about to advance
bucket).
There was a problem hiding this comment.
That's a nice analysis & it's exactly right -- in fact, I'll steal it to document the invariants!
This is unlikely to be a new algorithm -- I can swear I saw something like this done before, although maybe not in these exact circumstances. (Knuth, maybe? This feels like the sort of thing he may include as an exercise.)
We're experimenting with switching some forms of bridging from _DictionaryBuilder to this, but it's only an experiment at this point. This is designed specifically to work with the narrow use case of -[NSDictionary objects:keys:count:].
It's quite more fiddly than _DictionaryBuilder, so if the input comes from a sequence of unique key/value pairs, then I expect the builder will likely still be faster. (Although benchmarks may surprise us!)
There was a problem hiding this comment.
I wonder if there's a tiny simplification in going backwards instead of forwards—indeed, @Catfish-Man described it to me that way at first. That way you only have two regions to think about (and for a concrete benefit, only have to make one comparison).
|
@swift-ci test |
|
Build failed |
|
Build failed |
| // Discard existing entry, then move the current entry in place of it. | ||
| uncheckedDestroy(at: b) | ||
| _storage._count -= 1 | ||
| moveEntry(from: bucket, to: b) |
There was a problem hiding this comment.
Wouldn't it be cheaper to leave the existing entry where it is? Why does the first one win?
There was a problem hiding this comment.
This is how the existing bridging code works for String keys. Arguably the behavior is observable -- although you have to really work for it, so it probably isn't worth worrying about it.
There was a problem hiding this comment.
Ah, this is already breaking first/last ordering anyway because of all the swapping it does. I'll change it to discard the current entry.
|
@swift-ci smoke test |
|
Closing; this has landed in #21235. |
This in an experimental Dictionary initializer that is designed to work better with
-[NSDictionary getObjects:andKeys:count:]. This is primarily useful during bridging.(Interface inspired by @natecook1000's Array initializer proposal.)