[GODRIVER-1142] Improve decodeCommandOpMsg performance Created: 16/Jun/19 Updated: 26/Sep/19 Resolved: 26/Jun/19 |
|
| Status: | Closed |
| Project: | Go Driver |
| Component/s: | Wire Protocol |
| Affects Version/s: | 1.0.3 |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Łukasz Marszał | Assignee: | Kristofer Brandow (Inactive) |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||
| Description |
|
I was doing some performance tests of my app - in golang, using mongo-driver. After further investigation I've realized that decodeCommandOpMsg does decode full bson payload (all the way down) just to merge wiremessage sections toghether and return back as binary data (mainDoc.UnmarshalBSON and then mainDoc.MarshalBSON back again). My theory is that it doesn't need to bother about internal structure of a document whatsoever - just merge top-level fields. I've written simple benchmark and it seems it can be speeded up by the order of magnitude. You can see benchmarks code here: https://github.com/g2a-com/mongo-go-driver/commit/1cf2b874b5f8dd022c2b7aed7e9090256eb335d8?diff=unified Results - without a change:
Results - without my change (https://github.com/g2a-com/mongo-go-driver/commit/deeacb2e7aa5e9cd7820c52b4f945fdc5d09be52):
Fun fact is that without my change in most cases documents returned from mongodb would be unmarshaled twice - once in decodeCommandOpMsg and 2nd time when driver's user wants to read his data (e.g. by cursor.Unmarshal). Any thoughts? |
| Comments |
| Comment by Kristofer Brandow (Inactive) [ 26/Jun/19 ] | |||||||||||||||||||||||||||||||||||||||
|
lmarszal, no problem! Thanks for working with us on this. I'll close this ticket out. | |||||||||||||||||||||||||||||||||||||||
| Comment by Łukasz Marszał [ 24/Jun/19 ] | |||||||||||||||||||||||||||||||||||||||
|
kris.brandow - OK, now I see what do you mean. I didn't see x/network/command/opmsg.go is not being used on master branch. I ported my change back to v1.0.3 (this is where I did my original investigation): https://github.com/g2a-com/mongo-go-driver/tree/backport_opmsg_performance
Results:
This clearly shows that problem reported here is fixed on master. Sorry for the confusion. | |||||||||||||||||||||||||||||||||||||||
| Comment by Kristofer Brandow (Inactive) [ 21/Jun/19 ] | |||||||||||||||||||||||||||||||||||||||
|
lmarszal, The files you provided are using the old code. The new code is located here: https://github.com/mongodb/mongo-go-driver/blob/e4bd39d2c760255a391c8c0239833ee2ddb011c2/x/mongo/driver/operation.go#L1098-L1134. This code does minimal parsing and uses the bsoncore package. This shouldn't have the same performance issues as the code from the command package. | |||||||||||||||||||||||||||||||||||||||
| Comment by Łukasz Marszał [ 21/Jun/19 ] | |||||||||||||||||||||||||||||||||||||||
|
kris.brandow, everything is provided above and checked-in into https://github.com/g2a-com/mongo-go-driver/tree/opmsg_performance I was using data from mongo-go-driver repo (flat_bson.json and deep_bson.json).
Then, run make perf to download test data and you can run these benchmarks with commands provided above. If you want to test my improvements, download and replace also this file: https://github.com/g2a-com/mongo-go-driver/blob/deeacb2e7aa5e9cd7820c52b4f945fdc5d09be52/x/network/command/opmsg.go | |||||||||||||||||||||||||||||||||||||||
| Comment by Kristofer Brandow (Inactive) [ 21/Jun/19 ] | |||||||||||||||||||||||||||||||||||||||
|
lmarszal, can you please provide the full test files to reproduce? | |||||||||||||||||||||||||||||||||||||||
| Comment by Łukasz Marszał [ 21/Jun/19 ] | |||||||||||||||||||||||||||||||||||||||
|
esha.bhargava - I did benchmark on latest commit to master on github 4f95475. There are three more commits publically available since then (d4dabd4, 2b8bc9c and 24c422e) and benchmarking there gives same results (well... not a suprise as none these commits changes anything around bson or opmsg). | |||||||||||||||||||||||||||||||||||||||
| Comment by Esha Bhargava [ 20/Jun/19 ] | |||||||||||||||||||||||||||||||||||||||
|
lmarszal Can you try benchmarking on master? We have changed this code. |