Skip to content
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

Drastic performance difference between using SqlClient and SSMS #412

Closed
Skyppid opened this issue Feb 7, 2020 · 14 comments
Closed

Drastic performance difference between using SqlClient and SSMS #412

Skyppid opened this issue Feb 7, 2020 · 14 comments
Labels
📈 Performance Use this label for performance improvement activities

Comments

@Skyppid
Copy link

Skyppid commented Feb 7, 2020

We're currently saving a complex object structure in XML form. For safety and convenience we're using an SQL Express Server for handling the data. Right now we save the full document once every time changed (biggest being around 28MB of size) which should be no problem for SQL Server to handle.

The data is stored in a table with "Id (int, PK, Identity), Revision (int), Timestamp (datetime2), Data (xml)".
When I run the following query on SSMS (with Execution Plan), I see that it runs a clustered index seek which obviously doesn't take long for a table with ~600 rows. Using the SQL profiler I get the full XML data (non-trimmed) in 184ms (CPU: 47, Reads: 27044).

Now I do the very same thing in my application using plain SqlClient connection with the very same query: 36923ms (CPU: 47, Reads: 18286)

It's the exact same query when I look at the Profiler. Yet it takes ages longer until the query completes. And after that it also takes quite a while until ReadAsync() returns back with the XML data.

Can anybody explain to me why this happens? We currently have load times up to 6 minutes where as ~2.5 minutes are just retrieving the latest data from SQL Server.

As for the connection: The server is only running with TCP/IP protocols enabled.

@cheenamalhotra
Copy link
Member

Hi @Skyppid

Can you provide us a repro from System.Data.SqlClient or Microsoft.Data.SqlClient to capture slow transfer than we can look into?

@Skyppid
Copy link
Author

Skyppid commented Feb 18, 2020

What exactly do you mean, @cheenamalhotra?

@cheenamalhotra
Copy link
Member

I wanted to know more about your application, if you could confirm:

  • Does it target .NET Core/Framework? version?
  • OS details you're running this app on?
  • Have you tried to enable/disable MARS by any chance in your connection string, did it change behavior?

We also have another similar issue reported #422 you may wanna check, does it fall in the same case? If yes, we can combine both issues and continue investigations with 1 of them.

@Skyppid
Copy link
Author

Skyppid commented Feb 21, 2020

Ah okay. Well it does target both basically. The main application uses .Net Framework as it's currently an old WPF project (will be upgraded to .Net Core soon). The database and core libraries are set up as .Net Standard 2 libraries currently.

It's running on latest Windows 10, the SQL server either on that, too (at customers, local Express database) or on a Windows 2016 Server at our office.

MARS is currently disabled, though. Does take exactly as long with it enabled.

@cheenamalhotra
Copy link
Member

The database and core libraries are set up as .Net Standard 2 libraries currently.

I'm hoping this is where SqlClient is being referenced. Could you modify them to target .NET Framework, and confirm if you see performance gain?

Because .Net Standard loads from a different source (.NET Core driver), it's not the same as used by SSMS (which is .NET Framework driver), so it's likely leading to "read" performance issues in .NET Core driver.

@Skyppid
Copy link
Author

Skyppid commented Feb 28, 2020

I could but then I'd ask myself whether I should really upgrade to .Net Core if there are issues that severe... Nonetheless, it has changed nothing at all.

What I do see though in the time this query executes is that the GC is running like hell. I analyzed the heap and there are 3.023 TdsParserStateObject+PacketData objects on the heap totalling in ~25 MB memory. I see them less with the project targeting .Net Standard.

Anyway, still poor performance ... it takes way too long for 25MBs of data to retrieve from SQL Server.

@Wraith2
Copy link
Contributor

Wraith2 commented Feb 28, 2020

PacketData is use a lot with async reads. There was a particularly poor read performance with multipacket (i.e. long) strings which I made a fix for in #285 and that fix should be in the 2.0.0 preview. If you didn't try that one already can you do so and see if performance is improved? If perf hasn't improved if you can give me a representative minimal reproduction I can investigate the performance and see if there's any way to improve it.

@Skyppid
Copy link
Author

Skyppid commented Feb 28, 2020

Well first things first: Because .Net has become increasingly confusing, we actually had referenced System.Data.SqlClient instead of Microsoft.Data.SqlClient. I tried the stable 1.x and the latest 2.x and both performed equally poor.

What I did find out is that the issue is with ReadAsync(). When I call Read() instead, it succeeds within a fraction of a second. Since this is our biggest database and still that fast, it won't be an issue not using async/await.

But you should definetly look into it. It's weird that async execution takes that much longer. I can't really provide you with an sample as these XMLs contain sensitive information, though. But any big ~20-25MB XML file should do.

@Wraith2
Copy link
Contributor

Wraith2 commented Feb 28, 2020

I tried the stable 1.x and the latest 2.x and both performed equally poor.

That's very surprising, there's a huge perf improvement on .net core in Microsoft.Data.SqlClient 2.0 over 1.1 based on the pr I linked. If you can't replicate that improvement it makes me wonder if you're ending up using the wrong version again. I'd advise scrubbing System.Data references from the test project entirely. If you're on desktop framework then you're out of luck for the moment.

It's weird that async execution takes that much longer.

I did some fairly deep investigation and while surprising it's not unwarranted. It turns out async state management in the face of possible hostile user interaction has to be very carefully done. If I could get confirmation on some of the questions asked in #245 (comment) there may be a way to improve it (again on core only) but lacking that info the current state is as good as it's getting.

@Skyppid
Copy link
Author

Skyppid commented Feb 28, 2020

Okay... well the library using SQL is based on .Net Standard, the app running it is .Net Framework 4.7.2.
In the end avoiding async/await here does not have any performance impact except speeding it up a lot. So I go with this.

Still odd that it slows it down from 800ms loading time to ~30-40 seconds.

@Wraith2
Copy link
Contributor

Wraith2 commented Feb 28, 2020

In the end avoiding async/await here does not have any performance impact except speeding it up a lot.

If you don't need that thread to be available for other work while it's waiting for network packets then that's ok. If you're in a situation where many threads are competing for cpu time async would be better. Depends on your use case.

It might be possible to port the core fix to desktop. I haven't looked. I choose to contribute my time to core issues since I know that codebase better and it's where I wanted performance improvements.

@miroslavp
Copy link

Just want to mention that I had the same issue with SQL Express. I had a table with about 100 records in it. The table had a column of type nvarchar(max). I was fetching data asynchronously using EF Core. The execution time was either instant or 8-11 seconds running exactly the same query. Most of the time it was instantaneous, but once in a while it hangs for about 10 seconds. No other apps were using the Sql Express. Once I moved to SQL Standard the problem disappeared. Can't remember the EF Core version, but it was at least 3.0. The dbcontext and the entity classes were in a class library if that matters at all.

@DavoudEshtehari DavoudEshtehari added the 📈 Performance Use this label for performance improvement activities label Mar 25, 2022
@pantonis
Copy link

Setting the SET ARITHABORT ON; on the start of my query fixed my query which was timing-out using C# and executing in SSMS in < 1 sec using EF Core 7. I observed that it was choosing different execution plan without ARITHABORT ON

@cheenamalhotra
Copy link
Member

cheenamalhotra commented Jul 11, 2024

There are known performance issues with Async reads, closing as duplicate of #593

@cheenamalhotra cheenamalhotra closed this as not planned Won't fix, can't repro, duplicate, stale Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📈 Performance Use this label for performance improvement activities
Projects
Development

No branches or pull requests

6 participants