-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Comparatively slow execution of parameterless Oracle SQL SELECT query #1945
Comments
Firstly, this is going to be virtually impossible for me to do much with without runnable code with illustrative data that actually reproduces the scenario, as this looks massively contextual. However, if we generalize, there are 6 main causes of possible bottlenecks here:
Let's discuss each separately raw database performanceIt is great that you've looked at timings in SQL Developer - that helps us establish some baseline numbers, however: we need to be a little careful. I don't know about Oracle (I'm simply not an Oracle person), but if I compare to SQL Server: there are actually some connection-mode raw bandwidth issuesI don't know what your data is, but if it contains a lot of large CLOB/BLOB data, then yes fetching that can cause perceived performance problems; and tools like SSMS (and by extension, possibly SQL Developer) often don't fetch the entire payload by default - they select the first "some bytes", usually enough to fill a cell with a "bkdjhfghdfb..." with the "..." telling you "this is bigger". Fetching the full values obviously requires more network bandwidth and internal database IO. You might want to verify whether this is giving misleading timings in SQL Developer. Also, is the test machine the same here between the SQL Developer and .NET runs? (as obviously different machines may have different routes and network capacities) The fact that not selecting certain columns impacts the performance: makes me very suspicious that this might be a factor. I suspect that this is the most likely cause of the difference, honestly. internal buffering / resize overheadsI don't think this should be an issue, as it should reach a big size pretty efficiently; however, I wonder if we can try some things here. You mention that overheads in the ADO.NET provider etcI can't do anything about that; that would be an internal OracleCommand etc thing overheads inside DapperThere aren't a lot here; we've worked pretty hard to make things pretty efficient; one thing that might be interesting is to write the same code manually via raw ADO.NET and see if it behaves similarly, vs whether it is now magically faster. I suspect that if you tried this, you'd find that it behaves very similarly, but I can't do this without a runnable repro. The DapperAOT bits have some slightly more efficient data access API usage, which might be worth investigating but emphasis: this package is not ready for real usage. If I had a runnable sample, that's one thing I'd look at, purely as a quick way of finding things. framework overheadsAgain, outside of our control, but a possible cause of problems - in particular on the async path; so one thing I'd be keen to try is sync vs async; but I suspect there's no big problems here. Again, without runnable code it is hard to say much, but my suspicion (and it can be no more than that, without a runnable scenario) is that this is simply CLOB/BLOB bandwidth caused by the necessity of fetching the entire payloads. |
For completeness, one other possible factor: "sync context". The easiest way of ruling this out is to try sync vs async. If sync is just as slow: it isn't that! |
Many thanks for the response @mgravell. Acknowledged your point about the executable code. I'm hoping I can maybe run with your general guidance since getting a standalone package up and running is more of an undertaking than I can take on right now. To your points:
SELECT sysdate from dual;
set term off;
set feed off;
spool "c:\temp\viewquery.csv";
set sqlformat csv;
-- My query here --
spool off;
SELECT sysdate from dual; I confirmed that the CSV file has fully populated data. Curiously the I should note that the dB connection I'm using is especially bad. I'm testing against it for precisely that reason, because it's the one that's going to annoy people the most. Under better network conditions Dapper would undoubtedly perform better, but I think this helps amplify any deltas between native vs Dapper execution.
Maybe I missed it on your DapperAOT announcement issue, but what's the rough timeline for a stable v1? |
I did a doublecheck on the SQL Developer export wherein I opened the export CSV file in VSCode, wiped its contents, and watched the live refresh as the query executed. It does indeed complete with a full data set in about 12s. This is for a slightly altered timerange to ensure the query isn't cached. |
Re v1: it builds on new compiler features (not language features) in net8; so: not before net8 which should be in November. It still targets all platforms - you don't need to use net8 - you just need an updated compiler. |
I agree the main thing to try here is a raw ADO.NET version, to see if that is faster. |
Okay, it does seem to be something with the Oracle ADO.NET provider. When I swap things out and just loop through DbDataReader.ReadAsync() without even any action or mapping on the row results, the performance is every bit as slow when wrapped by Dapper. Same if I use the sync methods; and changing the fetch size doesn't make any difference either. I'm really mystified as to why there could be such a performance delta but I'll close this issue and won't trouble you further. Thanks for giving me some pointers to look into. |
I spoke too soon: fetch size does matter. |
We set InitialLONGFetchSize to -1, which is guidance we've been given previously; Dapper/Dapper/CommandDefinition.cs Line 161 in 136dc11
|
Reading docs, it sounds like |
not in Dapper, but in DapperAot, I have an experiment that works: [CommandProperty<OracleCommand>(nameof(OracleCommand.FetchSize), 1024)]
static void SomeCode(DbConnection connection, string bar)
{
_ = connection.Query<Customer>("def");
_ = connection.Query<Customer>("def", new { Foo = 12, bar });
}
public class Customer
{
public int X { get; set; }
public string Y;
public double? Z { get; set; }
} This works for any command-level instance values, and ends up emitting tweaks as you would expect: public override global::System.Data.Common.DbCommand GetCommand(global::System.Data.Common.DbConnection connection,
string sql, global::System.Data.CommandType commandType, object? args)
{
var cmd = base.GetCommand(connection, sql, commandType, args);
if (cmd is global::Oracle.ManagedDataAccess.Client.OracleCommand cmd0)
{
cmd0.FetchSize = 1024;
}
return cmd;
} Would this approach work? I can't guarantee that I'm going to backport this to Dapper vanilla (see #1909), but: it could possibly be done. |
Yeah, For starters, I think a global setting for Regarding the DapperAOT example, I like the prospect of being able to decorate an object with the fetch size to be applied to the associated query; that said, it would be a significant refactor to associate the queries statically with the POCO. FWIW, I currently encapsulate my queries inside an object that derives from an interface like this: /// <summary>
/// Represents a parameterized SQL query
/// </summary>
public interface ISqlContent
{
/// <summary>
/// Gets the parameterized query
/// </summary>
/// <returns>Parameterized query</returns>
string GetContent();
/// <summary>
/// Gets the parameter substitution dictionary
/// </summary>
/// <returns></returns>
IDictionary<string, object> GetParameters();
} The instance takes in all the arguments to be injected into the query, and I use reflection to create the parameter dictionary; the content and parameters are ultimately used with QueryAsync. In sum, I think I'm closer to the vanilla usage of Dapper ( |
Interesting. Configuration (and how/where to do it) is always fun. For Dapper vanilla, anything beyond "single global configuration" is probably going to be too hard to squeeze in. Curiously though, we already have a few options in DapperAOT even for your more nuanced scenario - for example, it is possible to write a custom "command factory" which can inspect the args and command, and do something based on, say, a known dictionary key or an interface your code knows about - so it can make any decisions it wants. Does global help at all, or is that not enough control? |
I'd be delighted with global control. :) |
Added DapperAOT example ^^^; in particular see https://github.com/DapperLib/DapperAOT/blob/main/test/Dapper.AOT.Test/Interceptors/GlobalFetchSize.input.cs - the reality is that DapperAOT has much better plugin points for this kind of ad-hoc request; I'll see if we can squeeze something into Dapper core, but: more complex to do; that is on top of https://github.com/DapperLib/DapperAOT/blob/main/test/Dapper.AOT.Test/Interceptors/CommandProperties.input.cs, which provides another mechanism (again in AOT) |
Understood; sorry to ask you to revisit the original Dapper when you're trying to move onto something newer and better. If you come to the conclusion that it's more effort than it's worth, please let me know ASAP as I'll need to figure an alternative to modulate the fetch size. Since Dapper AOT is on the horizon I think I can clone your code locally and put some hacks in it to get me by until November. |
I'm tempted to try and hack it in, regardless; I think I see a way to do it; my main reservation is actually the thought "this value should be contextual", but the way I figure: that applies to the existing default too, and that certainly isn't contextual - so maybe an override default is fine; let me have a stab at it later - I know where to look and what to watch out for |
this very much shows some of the problems the AOT push is trying to resolve, though; right now, hooking in any change like this: is hella awkward and requires lots of knowledge of ref-emit; if we can get the AOT work "done", then we get much better options, where ref-emit becomes a fond but distant memory, and where tweaks are simple and provably work |
@kriewall so... how do you feel about testing this? Reality is I'm simply not an Oracle person; in theory setting Are you in a position to build locally, or should I setup a myget deploy? |
@mgravell Totally on board with testing. Looks like the changes are on the fetchsize branch. I'll try to find time today and do the validation. |
@mgravell Looks like it works! The only thing I noticed is that the default value for That said, when I check the Let me know if there's anything else you'd like me to do from a validation perspective. Will this change be published to Nuget soon, or is there a way I can get my hands on a prerelease version? |
The -1 is deliberate; basically I don't want to hard code Oracle's default
(for multiple reasons), and `Nullable<T>` seems overkill, so I'm treating
any negative value as "don't assign" - hence it *should* be the case that
even if you leave it at -1, it works fine - just: we don't touch the
`FetchSize`
Does that make sense and agree with what you're seeing?
…On Thu, 17 Aug 2023, 14:39 kriewall, ***@***.***> wrote:
@mgravell <https://github.com/mgravell> Looks like it works! The only
thing I noticed is that the default value for SqlMapper.Settings.FetchSize
seems to be -1 which from an OracleCommand standpoint is an invalid entry:
[image: image]
<https://user-images.githubusercontent.com/38048622/261324780-6380d1ad-a78b-4ced-9452-25117adf1e60.png>
That said, when I check the OracleCommand.FetchSize value at the time of
usage it is equal to the default value of 131072. If I set a custom
SqlMapper.Settings.FetchSize value >0 at startup I see that value
reflected in OracleCommand.FetchSize when the command is constructed
within Dapper. Large FetchSize values dramatically improve the performance
of large queries.
Let me know if there's anything else you'd like me to do from a validation
perspective. Will this change be published to Nuget soon, or is there a way
I can get my hands on a prerelease version?
—
Reply to this email directly, view it on GitHub
<#1945 (comment)>
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAEHMHPSQSQPFY7JN6JCPLXVYNI3BFKMF2HI4TJMJ2XIZLTSOBKK5TBNR2WLJDUOJ2WLJDOMFWWLO3UNBZGKYLEL5YGC4TUNFRWS4DBNZ2F6YLDORUXM2LUPGBKK5TBNR2WLJDUOJ2WLJDOMFWWLLTXMF2GG2C7MFRXI2LWNF2HTAVFOZQWY5LFUVUXG43VMWSG4YLNMWVXI2DSMVQWIX3UPFYGLLDTOVRGUZLDORPXI6LQMWWES43TOVSUG33NNVSW45FGORXXA2LDOOJIFJDUPFYGLKTSMVYG643JORXXE6NFOZQWY5LFU4YTMMJTGM2DLAVEOR4XAZNFNFZXG5LFUV3GC3DVMWVDCOBUGY2TQMZTGA42O5DSNFTWOZLSUZRXEZLBORSQ>
.
You are receiving this email because you were mentioned.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>
.
|
Re NuGet: yes, we can get this merged quickly - probably Friday (I replied on the -1 thing by email; it should appear here soon? |
Clear on the -1 and its meaning. Seems reasonable and is consistent with the fact that the FetchSize is the Oracle default unless explicitly assigned. Appreciate your support on this! I'll stay tuned for the package push. |
As-is behavior:
First-time execution of Dapper's
QueryAsync<T>
extension method consistently takes over 1 minute to return when supplied with a parameterless Oracle SQL query that returns 21497 rows with 31 values each. The same query when executed in SQL Developer on the same machine with the same connection string takes 12s (In SQL Developer, spooling to file was used to avoid overly optimistic timing results resulting from pagination.)Expected behavior:
Dapper's
QueryAsync<T>
should return in roughly the same amount of time as the native query; no longer than 2X as a ballpark.Environment:
Code:
Argument values to QueryAsync:
Out of a total 1:06 execution time, the following block of code in SqlMapper.Async takes 1:02:
Stepping through this block via symbol file indicates no observable performance lag. No branch other than the first is ever hit in the
GetValue<T>
method:Below is a mapping of the properties of the sanitized POCO class used as the generic type for QueryAsync/Query to the corresponding value types from the Oracle query:
Below is the sanitized SQL query
Other observations:
QueryAsync
(not surprising since it is the same code under the hood; however this suggests it doesn't have to do with my POCO).Query<T>
(also leverages samewhile
loop as above)QueryAsync<T>
. CPU ranges between 20-30% and memory was flat at 43%.QueryAsync<T>
; however when I tried setting it to false withQuery<T>
, the performance is about the same.QueryAsync
scales exponentially with the data volume, although this is just a rough estimate.Specific inquiries:
Really hoping someone can assist. Dapper is deeply embedded in our code but the performance especially for large queries is becoming critical.
The text was updated successfully, but these errors were encountered: