[GODRIVER-1563] Investigate BSON performance Created: 08/Apr/20 Updated: 25/Jun/20 Resolved: 25/Jun/20 |
|
| Status: | Closed |
| Project: | Go Driver |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | David Bartley | Assignee: | Divjot Arora (Inactive) |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||
| Issue Links: |
|
||||
| Epic Link: | GODRIVER-1587 | ||||
| Description |
|
Compared to globalsign/mgo, mongo/bson still performs worse.
Tests run against HEAD of both: |
| Comments |
| Comment by David Bartley [ 25/Jun/20 ] | ||
|
Thanks for taking this on, that'd be fine! | ||
| Comment by Divjot Arora (Inactive) [ 25/Jun/20 ] | ||
|
Hi bartle, I'm getting around to working on this project. The full description is in GODRIVER-1587. These are the changes we're considering making:
I was planning on using https://github.com/mongodb/mongo-go-driver/compare/master...bartle-stripe:bartle-unmarshal-perf as the base for (1) and (3) if you're OK with that. For (3), I think we can pretty much use the DDecodeValue function you added, but I'm not sure yet if we also need a dDecodeType to implement typeDecoder. If you're OK with these suggestions, I can go ahead and close out this ticket and we can investigate further in GODRIVER-1587. – Divjot | ||
| Comment by Githook User [ 20/Apr/20 ] | ||
|
Author: {'name': 'David Bartley', 'email': 'bartle@stripe.com', 'username': 'bartle-stripe'}Message: | ||
| Comment by David Bartley [ 09/Apr/20 ] | ||
|
I have a couple branches: The first commit in both of those reduces allocs by directly appending to the underlying slice. The second one is a tad bit slower but I think it avoids some duplication present in the first approach (though I think the first is easier to understand). The second commit is much more involved. The existing ValueDecoder interface requires that everything passed to it be settable/addressable. This is reasonable when using the decoder on struct fields, but for a bson.D or empty interface where you don't have anything allocated up front it's not. Instead, this adds a new TypeDecoder interface that directly returns a new value. If the codec interfaces aren't considered to be stable, I'd probably just merge ValueDecoder and TypeDecoder, such that you'd just have "DecodeValue(DecodeContext, bsonrw.ValueReader, reflect.Type) (reflect.Value, error)". If the passed-in value is non-nil/settable, you'd mutate that value directly, but otherwise it'd look like DecodeType. Please let me know what you think of these approaches! | ||
| Comment by Divjot Arora (Inactive) [ 09/Apr/20 ] | ||
|
Hi bartle, Thank you for the in-depth investigation and analysis. Based on your previous comments, it seems like you've been able to improve the performance in three separate occasions. Can you link some code containing whatever changes you made to the driver's BSON library to get these improvements in whatever format in whatever format is most convenient for you? I'm leaving the ticket in "Needs Triage" for now so we can decide how to prioritize it in our next meeting, but the code would be super helpful so we don't end up re-creating work you've already done. | ||
| Comment by David Bartley [ 09/Apr/20 ] | ||
|
Indeed, if I change "RegisterTypeMapEntry" to register a "func(DecodeContext, bsonrw.ValueReader) (reflect.Value, error)" instead of "reflect.Type", and use that directly (instead of allocating a value and calling DecodeValue), it's significantly faster and has fewer allocs:
| ||
| Comment by David Bartley [ 09/Apr/20 ] | ||
|
I suspect we can do better. Right now, slice/bson.D allocates a pointer to the go type corresponding to the BSON type and passes that to DecodeValue. However, I suspect if we instead had a variant of DecodeValue that returned a reflect.Value we could avoid some allocations. | ||
| Comment by David Bartley [ 09/Apr/20 ] | ||
|
With some further optimizations:
| ||
| Comment by David Bartley [ 09/Apr/20 ] | ||
|
Implementing a type decoder for bson.D improves things:
| ||
| Comment by David Bartley [ 09/Apr/20 ] | ||
|
I realized that on the unmarshaling side this wasn't quite a fair test, because mgo will unmarshal to a map, whereas mongo will unmarshal to a bson.D. If I force unmarshalling to a bson.D, it's still slower:
|