Skip to content

Updates to aspnetcore PG database access #8005

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Mar 20, 2023
Merged

Updates to aspnetcore PG database access #8005

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0e7509d
Some code cleanup
roji Mar 9, 2023
9074110
Updates to aspnetcore PG database access
roji Mar 9, 2023
7fafda4
Stop UTF8 decoding/reencoding in Fortunes platform
roji Mar 10, 2023
812a137
Update Fortunes to use Razor templating
roji Mar 16, 2023
383d34c
Turn of SQL parsing/rewriting for Fortunes
roji Mar 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions frameworks/CSharp/aspnetcore/PlatformBenchmarks/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ public static async Task Main(string[] args)
{
Args = args;

#if NPGSQL
// This disables SQL parsing/rewriting, which requires using positional parameters and NpgsqlBatch everywhere.
// This helps commands where there are no parameters (Fortunes); when there are parameters, their ParameterName
// being null already triggers positional parameters and disables parsing)
// Note that Dapper and EF aren't yet compatible with this mode.
AppContext.SetSwitch("Npgsql.EnableSqlRewriting", false);
#endif
Comment on lines +17 to +23
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this something we have enabled in our copy of the benchmark, or is it not required there for other reasons?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't do it in our copy since we have Dapper and EF, which this is incompatible with... This one is tricky, since it's an AppContext switched cached at startup, but we don't know whether the application will be used for raw, dapper or EF.

This could be an #if (we already have #if DATABASE, #if NPGSQL vs. MYSQLCONNECTOR)...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Ideally this would be a property that be set directly on the command. What kind of gains are we talking about by setting it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See aspnet/Benchmarks#1590 (comment).

I did a measurement a long time ago and it wasn't very significant, but we're at the point where every little bit helps (I can measure again at some point).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using AppContext switches makes me slightly uneasy as they're usually reserved in .NET for restoring unsupported behaviors, rather than opting-in to legitimate optimizations. I think this is one of those things that's very much in the contentious or at least debatable zone WRT to keeping in the spirit of the benchmark.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed this offline and I'm OK with this going in now.


Console.WriteLine(Encoding.UTF8.GetString(BenchmarkApplication.ApplicationName));
#if !DATABASE
Console.WriteLine(Encoding.UTF8.GetString(BenchmarkApplication.Paths.Plaintext));
Expand Down