-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CSHARP-5807: Add benchmarks with strong-typed POCOs #1839
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds benchmarks that compare BsonDocument-based operations with strongly-typed POCO (Plain Old CLR Object) operations to measure the performance differences between the two approaches. The changes extend existing benchmarks across SingleDoc, MultiDoc, and Bson categories.
Key Changes:
- Added POCO data type definitions for benchmark data structures (SmallDocPoco, Tweet, FlatPoco, FullPoco, DeepPocoRoot)
- Extended existing benchmarks to include POCO variants alongside BsonDocument versions
- Refactored namespace declarations to file-scoped namespaces (first commit, not reviewed per description)
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| MultiDoc/MultiDocBenchmarkDataTypes.cs | New file defining POCO types (SmallDocPoco, Tweet, TweetEntities, User, UserMention) for multi-document benchmarks |
| Bson/BsonBenchmarkDataTypes.cs | New file defining POCO types (FlatPoco, FullPoco, DeepPocoRoot, DeepPocoNode) for BSON encoding/decoding benchmarks |
| SingleDoc/InsertOneSmallBenchmark.cs | Added InsertOneSmallPoco benchmark method and supporting POCO collection/data |
| SingleDoc/FindOneBenchmark.cs | Added FindOnePoco benchmark method with Tweet POCO support |
| MultiDoc/SmallDocBulkInsertBenchmark.cs | Added POCO variants for InsertMany and BulkWrite benchmarks |
| MultiDoc/FindManyBenchmark.cs | Added FindManyAndEmptyCursorPoco benchmark method |
| MultiDoc/BulkWriteMixedOpsBenchmark.cs | Added POCO variants for mixed bulk write operations |
| Bson/BsonEncodingBenchmark.cs | Added BsonEncodingPoco benchmark with POCO serialization |
| Bson/BsonDecodingBenchmark.cs | Added BsonDecodingPoco benchmark with POCO deserialization |
| Bson/BsonBenchmarkData.cs | Updated to include PocoType parameter for benchmark data |
| README.md | Minor punctuation fix (added comma) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for (int i = 0; i < Iterations; i++) | ||
| { | ||
| _client.Dispose(); | ||
| _collectionPoco.InsertOne(_smallDocumentPoco); |
Copilot
AI
Dec 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The InsertOneSmallPoco benchmark reuses the same _smallDocumentPoco instance for all 10,000 insertions. This will cause MongoDB to reject insertions after the first one due to duplicate _id errors (if the POCO has an Id property that gets set). The BsonDocument version correctly removes the _id field before each insertion. Consider either cloning the POCO or resetting its Id property before each insert.
| _collectionPoco.InsertOne(_smallDocumentPoco); | |
| var clonedPoco = BsonSerializer.Deserialize<SmallDocPoco>(BsonSerializer.Serialize(_smallDocumentPoco)); | |
| _collectionPoco.InsertOne(clonedPoco); |
| for (int i = 0; i < Iterations; i++) | ||
| { | ||
| _client.Dispose(); | ||
| _collection.Find(new BsonDocument("_id", i)).First(); |
Copilot
AI
Dec 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent use of discard assignment. The FindOne method doesn't use discard assignment for the result, but FindManyAndEmptyCursor does. For consistency, either add discard assignment to FindOne or remove it from the FindMany benchmarks (both in this file and FindOneBenchmark.cs).
| } | ||
|
|
||
| [Benchmark] | ||
| public void InsertManySmallBenchmarkPoco() |
Copilot
AI
Dec 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent naming convention for POCO benchmark methods. Some use "Poco" suffix (e.g., InsertOneSmallPoco, FindOnePoco) while others use "BenchmarkPoco" suffix (e.g., InsertManySmallBenchmarkPoco). Consider standardizing on one approach, such as using "Poco" suffix consistently across all benchmark methods.
| public void InsertManySmallBenchmarkPoco() | |
| public void InsertManySmallPoco() |
| _collection.BulkWrite(_collectionBulkWriteMixedOpsModels, new()); | ||
| } | ||
| [Benchmark] | ||
| public void SmallDocCollectionBulkWriteMixedOpsPocoBenchmark() |
Copilot
AI
Dec 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent naming convention for POCO benchmark method. This method uses "PocoBenchmark" suffix while other methods in the codebase use either "Poco" or "BenchmarkPoco" suffixes. Consider using "BenchmarkPoco" for consistency with other collection bulk write methods.
| public void SmallDocCollectionBulkWriteMixedOpsPocoBenchmark() | |
| public void SmallDocCollectionBulkWriteMixedOpsBenchmarkPoco() |
| var documents = Enumerable.Range(0, 10000) | ||
| .Select(i => _tweetDocument.DeepClone().AsBsonDocument.Add("_id", i)); | ||
| _collection.InsertMany(documents); | ||
| _collection.Find(new BsonDocument("_id", i)).First(); |
Copilot
AI
Dec 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The FindOnePoco benchmark is using the wrong collection. It should use _collectionPoco instead of _collection to properly benchmark POCO deserialization performance. Currently, it's benchmarking the same thing as the FindOne method.
| _collection.Find(new BsonDocument("_id", i)).First(); | |
| _collectionPoco.Find(new BsonDocument("_id", i)).First(); |
|
|
||
| private static void WriteScore(StreamWriter writer, string benchName, double score) | ||
| { | ||
| writer.WriteLine(score != 0 |
Copilot
AI
Dec 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Equality checks on floating point values can yield unexpected results.
The first commit is just a namespaces refactoring. Please review the second commit.