SWIFT-1070 Improve performance of BSONDecoder #54
Conversation
| return .array(doc.values) | ||
| var values: [BSON] = [] | ||
| let it = doc.makeIterator() | ||
| while let (_, val) = try it.nextThrowing() { |
There was a problem hiding this comment.
since BSONDocument.read doesn't validate anymore, we need to use nextThrowing to ensure we don't fatalError. Full validation will be performed in Array.validate if necessary.
| public func map<T>( | ||
| _ transform: (Element) throws -> T | ||
| ) rethrows -> [T] { | ||
| try AnySequence(self).map(transform) |
There was a problem hiding this comment.
This was calling count a lot, which now requires a full walk over the document. To avoid any issues, I just did a naive manual implementation of map.
There was a problem hiding this comment.
LOL I was looking at the method body but not the actual context of what method you were implementing here and I was like "... why isn't he just calling map?" duh, because you are implementing map 🤦♀️
| default: | ||
| try doc.set(key: self.keys[i], to: value.bson) | ||
| } | ||
| doc.append(key: self.keys[i], value: value.bson) |
There was a problem hiding this comment.
since we manually handle overwriting keys in the keyed encoding, we can just append directly without checking prior bytes of the document. This means we don't have to throw in here anymore and can use the .bson computed property.
|
Oh I forgot to mention: I left off the improvements to |
|
|
||
| internal init(keyValuePairs: [(String, BSON)]) { | ||
| self.keySet = Set(keyValuePairs.map { $0.0 }) | ||
| guard self.keySet.count == keyValuePairs.count else { |
There was a problem hiding this comment.
should we still be making this assertion, even if we don't store the key set? I think without this we might allow a user to do something like let doc: BSONDocument = ["a": 1, "a": 2] since the dictionary literal init delegates to this method.
There was a problem hiding this comment.
good catch. Since this is primarily a concern of dictionary literal initializers, I've moved the validation into them instead of here, since this initializer is also used elsewhere (on potentially un-validated documents in the future).
| public func map<T>( | ||
| _ transform: (Element) throws -> T | ||
| ) rethrows -> [T] { | ||
| try AnySequence(self).map(transform) |
There was a problem hiding this comment.
LOL I was looking at the method body but not the actual context of what method you were implementing here and I was like "... why isn't he just calling map?" duh, because you are implementing map 🤦♀️
2 participants
SWIFT-1070
This PR improves the performance of the
BSONDecoder. The goal of this PR is simply to match the libbson-based BSON library's performance, so there are still more possible improvements to be done in the future.We actually ended up improving upon the existing libbson-based
BSONDecoderpretty significantly:BSON to Native Benchmark Results
We also improved upon the improved Native to BSON benchmarks we originally saw in
swift-bson:Native to BSON Benchmark Results