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

Improvement the time to execute parametred queries [DNET604] #566

Closed
firebird-automations opened this issue Apr 24, 2015 · 6 comments
Closed

Comments

@firebird-automations
Copy link

Submitted by: Pascal (webpac)

When FirebirdSql.Data.FirebirdClient is used to insert a lot of records in a table using parametred query, it's possible to gain time in the FbCommand.UpdateParameterValues :

Change this code :
for (int i = 0; i < this.statement.Parameters.Count; i++)
{
index = i;

            if \(this\.namedParameters\.Count \> 0\)
            \{
                    index = this\.Parameters\.IndexOf\(this\.namedParameters\[i\]\);
                    if \(index == \-1\)
                    \{
                        throw new FbException\(String\.Format\("Must declare the variable '\{0\}'", this\.namedParameters\[i\]\)\);
                    \}
            \}

by this one :
for (int i = 0; i < this.statement.Parameters.Count; i++)
{
index = i;

            if \(this\.namedParameters\.Count \> 0\)
            \{
                if \(this\.Parameters\[i\]\.ParameterName \!= this\.namedParameters\[i\]\) // Added by webpac
                \{
                    index = this\.Parameters\.IndexOf\(this\.namedParameters\[i\]\);
                    if \(index == \-1\)
                    \{
                        throw new FbException\(String\.Format\("Must declare the variable '\{0\}'", this\.namedParameters\[i\]\)\);
                    \}
                \}
            \}

And then, if the namedParamers[i] is the same then Parameters[i], it doesn't looking for all nameParameters.

I tried to insert 1000 records with 50 parameters, without the optimisation, it dures 10 seconds and with the optimisation it dures 7 seconds.

Regards,

Commits: ac85e41

@firebird-automations
Copy link
Author

Commented by: @cincuranet

Maybe it would be even better to build a dictionary for it and look up in it. Haven't you tried that?

@firebird-automations
Copy link
Author

Commented by: Pascal (webpac)

I didn't look at all the source code to understand the différences between parameters and namedparameters.
I think it's possible that is the only performance issue when trying to find the good parameter.
But it's possible that using a dictionary will guard to do possible others performance issues so it's a good idea to use a dictionary.

Regards,

@firebird-automations
Copy link
Author

Modified by: @cincuranet

status: Open [ 1 ] => In Progress [ 3 ]

@firebird-automations
Copy link
Author

Commented by: @cincuranet

Would you mind trying this (https://ci.appveyor.com/project/cincura_net/netprovider/build/0.0.0.171) build? I made few changes internally.

Still not sure the dictionary is worth it, because the method is called with every execution and hence the dictionary will be recreated every time.

Your check is good idea for fast-path. But needs to respect the logic of IndexOf, hence easy to break. Maybe now, with the internal changes, it will be OK.

@firebird-automations
Copy link
Author

Modified by: @cincuranet

status: In Progress [ 3 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

Fix Version: vNext [ 10704 ]

@firebird-automations
Copy link
Author

Commented by: Pascal (webpac)

I got "Project not found or access denied." when I click on your link, so I can't try it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants