CSHARP-5321: Optimize client-side projections to only fetch the parts of the document that are needed for the projection. #1589

rstam · 2025-01-09T00:07:42Z

No description provided.

rstam · 2025-01-09T00:08:57Z

src/MongoDB.Driver/ClientSideProjectionSnippetsDeserializer.cs

+            typeof(ClientSideProjectionSnippetsDeserializer<,,,,>)
+        ];
+
+        public const int MaxNumberOfSnippets = 4; // could be expanded up to 16


I stopped at 4 because I wanted to get farther in the review process before adding support for more snippets.

And we need to decide how many to add support for. The max would be 16 (based on the Func types available).

rstam · 2025-01-09T00:09:58Z

src/MongoDB.Driver/ClientSideProjectionSnippetsDeserializer.cs

+            var deserializerGenericTypeDefinition = __deserializerGenericTypeDefinitions[snippetTypes.Length];
+            var deserializerGenericTypeArguments = snippetTypes.Append(projectionType).ToArray();
+            var deserializerType = deserializerGenericTypeDefinition.MakeGenericType(deserializerGenericTypeArguments);
+            return (IBsonSerializer)Activator.CreateInstance(deserializerType, [snippetDeserializers, projector]);


Note: I decided it is better to use reflection ONCE to create the deserializer rather than to use reflection to call the projector delegate. That is because the projecter delegate has to be called for EVERY document returned from the server.

rstam · 2025-01-09T00:11:23Z

src/MongoDB.Driver/ClientSideProjectionSnippetsDeserializer.cs

+using MongoDB.Bson.Serialization;
+using MongoDB.Bson.Serialization.Serializers;
+
+namespace MongoDB.Driver


A "snippet" is the name I have given to a piece of the projector expression that can be evaluated server-side. The rest of the projecction is done client-side.

When finding the snippets we try to keep as much of the work as possible server-side which more often than not will reduce the amount of data sent to the client.

rstam · 2025-01-09T00:12:18Z

src/MongoDB.Driver/Linq/Linq3Implementation/Ast/Expressions/AstExpressionExtensions.cs

@@ -35,6 +35,9 @@ public static bool IsInt32Constant(this AstExpression expression, out int value)
        public static bool IsMaxInt32(this AstExpression expression)
            => expression.IsInt32Constant(out var value) && value == int.MaxValue;

+        public static bool IsRootVar(this AstExpression expression)


This test was performed so many times that I created a method to centralize the implementation and simplify the places where the test is needed.

rstam · 2025-01-09T00:14:08Z

src/MongoDB.Driver/Linq/Linq3Implementation/Ast/Optimizers/AstGroupingPipelineOptimizer.cs

                {
                    var rewrittenArg = (AstExpression)AstNodeReplacer.Replace(node.In, (node.As, _element));
                    var accumulatorExpression = AstExpression.UnaryAccumulator(AstUnaryAccumulatorOperator.Push, rewrittenArg);
                    var accumulatorFieldName = _accumulators.AddAccumulatorExpression(accumulatorExpression);
-                    var root = AstExpression.Var("ROOT", isCurrent: true);
-                    return AstExpression.GetField(root, accumulatorFieldName);
+                    return AstExpression.GetField(AstExpression.RootVar, accumulatorFieldName);


AstExpression.RootVar is the way we should always refer to the variable for $$ROOT.

By using AstExpression.RootVar we ensure that the variable is created the same way every time (I found a few places where it was not). A secondary benefit is that there is only one instance of the RootVar that can be re-used over and over.

rstam · 2025-01-09T00:22:43Z

...plementation/Translators/ExpressionToPipelineTranslators/SelectMethodToPipelineTranslator.cs

                }

-                return pipeline;
+                return projectStage == null ?


projectStage will be null when the client-side projection needs the whole document as input.

rstam · 2025-01-09T00:23:50Z

tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Jira/CSharp3529Tests.cs

-                AssertStages(stages, Array.Empty<string>());
-                serializer.Should().BeOfType<ClientSideProjectionDeserializer<DocumentWithArray, int>>();
+                var stages = Translate(collection, queryable, out var outputSerializer);
+                AssertStages(stages, "{ $project : { _0 : '$A', _id : 0 } }");


A lot of tests involving client-side projections had to be modified to pass now that we have optimized client-side projections.

rstam · 2025-01-09T00:24:34Z

tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Jira/CSharp5321Tests.cs

+                .Select(x => Add(x.X, 1));
+
+            var stages = Translate(collection, queryable);
+            AssertStages(stages, "{ $project : { _0 : '$X', _id : 0 } }");


Note that the constant 1 is not a snippet. It remained client-side.

rstam · 2025-01-09T00:25:04Z

tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Jira/CSharp5321Tests.cs

+                .Select(x => Add(x.A.Sum(), 4));
+
+            var stages = Translate(collection, queryable);
+            AssertStages(stages, "{ $project : { _0 : { $sum : '$A' }, _id : 0 } }");


Note that the sum is done server-side, avoiding fetching a potentially large array to the client.

rstam · 2025-01-09T00:25:45Z

src/MongoDB.Driver/Linq/Linq3Implementation/Misc/LambdaExpressionExtensions.cs

@@ -38,7 +36,7 @@ public static string TranslateToDottedFieldName(this LambdaExpression fieldSelec
            {
                throw new ArgumentException($"ValueType '{parameterSerializer.ValueType.FullName}' of parameterSerializer does not match parameter type '{parameterExpression.Type.FullName}'.", nameof(parameterSerializer));
            }
-            var parameterSymbol = context.CreateSymbolWithVarName(parameterExpression, varName: "ROOT", parameterSerializer, isCurrent: true);
+            var parameterSymbol = context.CreateRootSymbol(parameterExpression, parameterSerializer);


Creating a root symbol is done often enough that I have added a new CreateRootSymbol helper method to help ensure that the root symbol is always created correctly.

rstam · 2025-01-10T19:39:41Z

Some tests are failing on pre 4.4 servers. I'm investigating.

rstam · 2025-01-10T20:29:00Z

I have made minor changes so that this change works on older servers also.

rstam · 2025-01-10T21:41:00Z

Looks like now there are some other tests failing pre 4.4 servers... I'll probably just ending up skipping them on older servers since very soon 4.4 will be the minimum supported server version.

rstam · 2025-01-10T21:55:49Z

tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Jira/CSharp4410Tests.cs

@@ -180,6 +182,7 @@ public void Select_with_enum_comparison_should_work(int i, string projectionAsSt
        public void Comparison_of_enum_and_enum_with_mismatched_serializers_should_throw(
            [Values(false, true)] bool enableClientSideProjections)
        {
+            RequireServer.Check().Supports(Feature.FindProjectionExpressions);


I'm skipping most tests involving client-side projections on older servers.

The only tests that have special assertions for pre 4.4 servers are CSHARP-4763 and CSHARP-5321 (this one).

rstam · 2025-01-10T21:57:09Z

tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Jira/CSharp5321Tests.cs

+            var stages = Translate(collection, queryable);
+            if (translationOptions.CompatibilityLevel == ServerVersion.Server42)
+            {
+                stages.Should().BeEmpty();


Pre 4.4 servers don't support expressions in Find projections, so on those older servers we just fetch the whole document and pass the whole document to the client-side projection function.

sanych-sun

LGTM

adelinowona · 2025-01-16T10:07:09Z

LGTM

rstam · 2025-01-23T18:10:26Z

...n/Translators/ExpressionToAggregationExpressionTranslators/ClientSideProjectionTranslator.cs

+
+namespace MongoDB.Driver.Linq.Linq3Implementation.Translators.ExpressionToAggregationExpressionTranslators
+{
+    internal static class ClientSideProjectionTranslator


Note that I renamed ClientSideProjectionRewriter to ClientSideProjectionTranslator to avoid a naming conflict.

rstam · 2025-01-23T18:13:00Z

...n/Translators/ExpressionToAggregationExpressionTranslators/ClientSideProjectionTranslator.cs

+            LambdaExpression projectionLambda,
+            IBsonSerializer sourceSerializer)
+        {
+            var (snippets, rewrittenProjectionLamdba) = ClientSideProjectionRewriter.RewriteProjection(context, projectionLambda, sourceSerializer);


We now get the snippets and the rewritten lambda in one pass.

rstam · 2025-01-23T18:14:06Z

...ion/Translators/ExpressionToAggregationExpressionTranslators/ClientSideProjectionRewriter.cs

+
+namespace MongoDB.Driver.Linq.Linq3Implementation.Translators.ExpressionToAggregationExpressionTranslators
+{
+    internal class ClientSideProjectionRewriter: ExpressionVisitor


Note that I renamed ClientSideProjectionSnippetsTranslator to ClientSideProjectionRewriter because it now rewrites the lambda at the same time that it is finding and translating the snippets.

rstam · 2025-01-23T18:15:28Z

...ion/Translators/ExpressionToAggregationExpressionTranslators/ClientSideProjectionRewriter.cs

+            return base.VisitMethodCall(node);
+        }
+
+        private Expression VisitThenBy(MethodCallExpression node)


Before we could just skip over OrderBy and ThenBy but now that we are rewriting the lambda we have to visit them in a special way that doesn't split them across the client/server boundary.

rstam · 2025-01-23T18:16:31Z

src/MongoDB.Driver/Linq/LinqProviderAdapter.cs

            ExpressionTranslationOptions translationOptions,
-            AstSimplifier simplifier)
+            bool forFind)


This one parameter replaces projectionCreator and simplifier because the value of forFind implies the values for the other two so we only need this one parameter instead of three.

I feel like a better name might be clearer but can't think of one.

withinFindProjection perhaps?

rstam · 2025-01-23T18:20:16Z

src/MongoDB.Driver/Linq/LinqProviderAdapter.cs

+                IBsonSerializer projectionSerializer;
+
+                var wireVersion = context.TranslationOptions.CompatibilityLevel.ToWireVersion();
+                if (forFind && !Feature.FindProjectionExpressions.IsSupported(wireVersion))


This seems to be the best place to check whether the projection is forFind and whether the CompatibilityLevel indicates that we can use aggregation expressions in find projections.

damieng

Nothing major here to address but worth considering a few more tests.

Love the solution and approach!

damieng · 2025-01-24T17:01:48Z

tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Jira/CSharp5321Tests.cs

+            var result = queryable.Single();
+            result.Should().Be(10);
+        }
+


I feel like we should probably have more tests here?

Multiple snippets

Where the snippets have resolved out to a single get everything

Where it simply can't project at all

sanych-sun

LGTM

adelinowona

LGTM!

…ossible of the projection on the server.

rstam requested review from damieng, sanych-sun and adelinowona January 9, 2025 00:07

rstam requested a review from a team as a code owner January 9, 2025 00:07

rstam commented Jan 9, 2025

View reviewed changes

rstam commented Jan 10, 2025

View reviewed changes

rstam force-pushed the csharp5321 branch from d175be3 to 34ca6d3 Compare January 14, 2025 18:58

sanych-sun reviewed Jan 14, 2025

View reviewed changes

rstam force-pushed the csharp5321 branch from 34ca6d3 to 782ad86 Compare January 22, 2025 00:33

rstam commented Jan 23, 2025

View reviewed changes

rstam requested a review from sanych-sun January 23, 2025 18:21

damieng approved these changes Jan 24, 2025

View reviewed changes

rstam requested review from papafe and removed request for papafe January 28, 2025 16:42

sanych-sun approved these changes Jan 29, 2025

View reviewed changes

adelinowona approved these changes Jan 31, 2025

View reviewed changes

rstam force-pushed the csharp5321 branch 2 times, most recently from f71b616 to bddfa67 Compare February 6, 2025 01:41

CSHARP-5321: Optimize client-side projections to perform as much as p…

836aa60

…ossible of the projection on the server.

rstam force-pushed the csharp5321 branch from bddfa67 to 836aa60 Compare February 6, 2025 16:12

rstam merged commit 836aa60 into mongodb:main Feb 6, 2025
1 check was pending

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CSHARP-5321: Optimize client-side projections to only fetch the parts of the document that are needed for the projection. #1589

CSHARP-5321: Optimize client-side projections to only fetch the parts of the document that are needed for the projection. #1589

rstam commented Jan 9, 2025

rstam Jan 9, 2025

rstam Jan 9, 2025

rstam Jan 9, 2025

rstam Jan 9, 2025

rstam Jan 9, 2025

rstam Jan 9, 2025

rstam Jan 9, 2025

rstam Jan 9, 2025

rstam Jan 9, 2025

rstam Jan 9, 2025

rstam commented Jan 10, 2025

rstam commented Jan 10, 2025

rstam commented Jan 10, 2025

rstam Jan 10, 2025

rstam Jan 10, 2025

sanych-sun left a comment

adelinowona commented Jan 16, 2025

rstam Jan 23, 2025

rstam Jan 23, 2025

rstam Jan 23, 2025

rstam Jan 23, 2025

rstam Jan 23, 2025

damieng Jan 24, 2025

damieng Jan 24, 2025

rstam Jan 23, 2025

damieng left a comment •

edited

Loading

damieng Jan 24, 2025

sanych-sun left a comment

adelinowona left a comment

CSHARP-5321: Optimize client-side projections to only fetch the parts of the document that are needed for the projection. #1589

CSHARP-5321: Optimize client-side projections to only fetch the parts of the document that are needed for the projection. #1589

Conversation

rstam commented Jan 9, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rstam commented Jan 10, 2025

rstam commented Jan 10, 2025

rstam commented Jan 10, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanych-sun left a comment

Choose a reason for hiding this comment

adelinowona commented Jan 16, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damieng left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanych-sun left a comment

Choose a reason for hiding this comment

adelinowona left a comment

Choose a reason for hiding this comment

damieng left a comment •

edited

Loading