Heavy GC thrashing in TdsParser #2120
Comments
|
The array rental path is working. The problem is that there is so much GC activity that the array pool keeps getting cleaned up so reuse of the array pool arrays is limited to the time periods between GC's. My advice with current SqlClient builds is to not read strings over a single packet in size in async mode. Try using a sync api. |
|
It doesn't appear to be related. I'm delving into what's happening. |
|
I'm wrong, It is related. Sort of. The async read issue in #593 is magnifying and causing extra GC pressure but it isn't the root cause I don't think. It feels like there should be some way to feed data from the last replay of a failed column read into next one so we can start with an approximately correctly sized buffer and relieve some of the pressure. The reason it's doing so much allocation in the first place is that it's a plp read so we get chunks of data but don't know how much we're going to have in total so we can't allocate enough space upfront. |
|
When the code is called synchronously the rented buffer size doubles unitl it has grown big enough to finish the request. Sync: But when the code is called asynchronously the Maybe the offset could be stored somewhere for the duration of request to avoid this? |
|
I investigated that. We do something like that for byte arrays. After considering it for a while I'd rather not do it for 2 reasons. 1) it makes the async snapshot/replay mechanism slightly more complex and 2) the real fix is not to replay the snapshot on every additional packet. I'd prefer to fix the deeper problem caused by #593 and then consider if it's sensible to memoize max sizes per field per row in snapshots by generalizing the current approach we're using for byte[]'s |
|
It seems the fix ( #2121 ) reduced GC pressure by a lot! At least in the sample application almost all allocations are gone! Before: After I will test with the real application next! |
|
I advise waiting for the CI on that PR and try the artifacts from it if it ends up green (or the failures are just infrastructure issues). |
|
Hey, is it possible to have a new preview release of 5.2? It would be nice to have this issue patched |
|
@Havunen next preview release is next mount. Date TBD. |
|
@JRahnama Its September now, any chance for a new release? Or preview release please ? This is important issue for us |
|
Tentative date for 5.2 preview 4 is 21 Sept. A few hot internal issues could push that out, though. 😞 |
|
Any news about the release? |
Yeah, I'm wondering about that myself. Just came back to this after seeing we're still getting massive amounts of GC thrashing for an issue I thought was taken care of almost a year ago, and I found this issue. According to dotMemory, it doesn't appear that the array pool is being used at all, even in the latest 5.2 preview: Any idea when we'll see a new preview drop? |
|
If you just want to check if it fixes your case you can use the artifacts from the CI run on the linked PR. Or build your own locally and try it. |
|
@Wraith2 Just checked out the code and built it locally. Interesting result: the
What's going on there? |
|
The test above is using large strings which are being allocated on the LOH, their presence is causing gen2 GC's. Your test should be running faster. If it isn't then provide me with your reproduction setup or i'm stuck with guessing. |
|
@Wraith2 It's the same test as the original issue. (The numbers I posted come from iterating the entire database, whereas the test case given there is of just one table, but if you run that test case you should see the issue pretty clearly.) |
|
Did you run your test in release mode? |
|
@Havunen ...no. Good catch. Let me rebuild and get back to you with results. |
|
@Havunen That fixed it. Thanks! |
|
Yeah only string concationations in that method are debug statements. |
|
@David-Engel @JRahnama Can we have a new release of Microsoft.Data.SqlClient please, so we could get rid off this problem in our application? Even preview release could do. Is there some reason why new preview version can't be released? Could those rest of the issues be released in the next preview instead? We have been waiting for almost two months for this performance bug to get fixed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
Fetching a row from database which has varchar(max) typed column and contains about 1Mb of text causes ~8GB of garbage collector pressure.
I believe this issue is related to: #1864
As you can see from the screenshot of our real application this single method of this library dominates the memory usage:
Exception message:
Stack trace (v5.2.0-preview3.23201.1):
To reproduce
I created a public sample console application where this issue can be reproduced
The situation is better in v5.2.0-preview3.23201.1 but in my opinion its still not good enough, is there anything else what can we done here to reduce the memory allocations?
https://github.com/Havunen/MicrosoftSqlDriverGcBug
Expected behavior
As little memory is allocated as possible, allocating gigabytes of memory for 1 megabyte of string sounds wrong.
Further technical details
Microsoft.Data.SqlClient version 5.1.1
.NET 6.0.21
SQL Server version: SQL Server 2019, 150
Operating system: Windows 11
The text was updated successfully, but these errors were encountered: