-
Notifications
You must be signed in to change notification settings - Fork 2k
Update dotnet version #9376
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
Update dotnet version #9376
Conversation
| <PackageReference Include="Npgsql" Version="8.0.5" /> | ||
| <PackageReference Include="MySqlConnector" Version="2.3.7" /> | ||
| <PackageReference Include="Dapper" Version="2.1.35" /> | ||
| <PackageReference Include="RazorSlices" Version="0.7.0" Condition="$(PublishAot) != 'true'" /> |
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.
Shouldn't the RazorSlices version be 0.8.1, like in Minimal.csproj?
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.
Not yet ... 0.8.1 doesn't support IBufferWriter anymore, I have started a PR to add the support back. So for now I stay on this previous version. The newer version will also allow us to remove conditional directives for AOT.
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.
I see.
| @@ -1,4 +1,4 @@ | |||
| { | |||
| "ConnectionString": "Server=tfb-database;Database=hello_world;User Id=benchmarkdbuser;Password=benchmarkdbpass;SSL Mode=Disable;Maximum Pool Size=18;NoResetOnClose=true;Enlist=false;Max Auto Prepare=4;Multiplexing=true;Write Coalescing Buffer Threshold Bytes=1000", | |||
| "ConnectionString": "Server=tfb-database;Database=hello_world;User Id=benchmarkdbuser;Password=benchmarkdbpass;SSL Mode=Disable;Maximum Pool Size=512;NoResetOnClose=true;Enlist=false;Max Auto Prepare=4", | |||
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.
Is this faster than a Maximum Pool Size of 256 (or 128), but with Multiplexing enabled?
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.
I did some local benchmarks and for 56 cores multiplexing is slower on dotnet.
Also got slightly better results with 512 connections in the pool. Happens to be the number of connections from wrk.
I noticed other dotnet benchmarks on TE were already doing this, I am late to the party ...
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.
Interesting, eager to see the results once this PR is merged.
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.
It's not great unfortunately, because multiplexing is supposed to make things faster, here we are just ignoring this advantage because what seems to be a lock contention at the db driver level which is exacerbated by using more cores. The good thing is that the new hardware is exposing this issue and we can work on it which will benefit everyone.
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.
/cc @NinoFloris who is working on some stuff related to this.
Yeah, multiplexing is implemented internally via a single write loop, which we knew would be a contention bottleneck given enough cores - this seems have been reached now.
No description provided.