IBX - INSERT performance

[GaborBoros]

Hi,

I use IBSQL for inserts and got 30 percent more performance with small modifications and ParamByNameEx. Please review and comment.

Gabor

[tonyw]

Thanks Gabor for pointing this out - although I am slightly surprised that you can get this performance boost from what appears to be a different way of indexing an array.

What has interested me is as to why the param setters in TIBXSQLVAR iterate round in this way checking"if FParent.FNames = FName then " when the param getters just make direct access to the FXSQLVAR. It seems to be covering a case where two params may have the same name/alias - which I would have thought was an error.

I will think about this one.

[tonyw]

Gabor,

To get a handle on what you have proposed, I have done a diff (attached) between your proposed changes and the current ibsql.pas.

1) Your main proposed change is to modify the TIBSQL param setters to change the way the param placeholder name is looked up and to add a "break" clause to the loop. The "break" is probably where most of your performance gain comes from.

The problem with the "break" is that this won't work when two SQL statement placeholders have the same name ( a point I overlooked in my quick reply). IBX allows statements such as:

Select * From TABLEA Where Key = :KEYVALUE
UNION
Select * From TABLEB Where Key = :KEYVALUE

In use, you set the value of the placeholder once only e.g.

IBSQL1.ParamByName('KEYVALUE').AsInteger := 1;

This sets both occurrences. However, if you add the "break" then only the first will be set and then only way to set the second one is to use a positional parameter. I thus can't accept the "break" as this would break a lot of implementations - including my own.

I have tried to think about whether there is a more performant way of handling multiple placeholders with the same name - but so far no luck.

2) You have also changed the loop check in the param setter from

"if FParent.FNames[\i] = FName then"

to

"if FParent[\i].FName = FName then".

In the former case, a TStringList is indexed and returns the corresponding string. In the latter case (your change) the object (a TIBXSQLDA) default indexed property is looked up to return the FName property of the TIBXSQLVAR. In both cases, the index range is checked and both return the same value as they are both set by TIBXSQLDA.AddName. I really can't see any performance difference between the two approaches - and if there was then there probably is a significant problem with TStringList. I can't see any good reason to accept the change.

3) You have added a new "ParamByNameEx" to provide an alternative to TIBSQL.ParamByName. There seem to be two differences from ParamByName. The first is that it will "prepare" the statement if not already prepared and the second is that  the placeholder name lookup is done directly on the TIBXSQLVAR rather than via the TStringList.

When I reviewed IBX in preparation for IBX for Lazarus, I did think about adding an automatic "prepare" to TOBSQL.ParamByName, given that I make a lot of direct use of TIBSQL and am bored with typing "prepare". On the other hand, it is a low level object. Most users use TIBCustomDataset (or descendents) and these call "Prepare" themselves. Adding "auto prepare" adds a small additional overhead and if you are going to make direct use of TIBSQL then you are probably looking for performence - hence I left it as it was.

With the direct lookup of the parameter name, you have missed out a call to "FormatIdentifierValue". This is an important omission as this function reformats the placeholder name according to the current SQL Dialect. Your "ParamByNameEx" should only work when the placeholder names you use are not reformatted by "FormatIdentifierValue". This is probably not a good idea.

Thanks for trying to improve IBX. I am always open to suggestions and it was interesting to go back and review how this part of the code works.

Tony

[Mike.Cornflake]

Completely unrelated reply to what is a fascinating subject.  If you'd wrapped your code in [ code ] [ /code ] tags, you could have written

"if FParent.FNames[\i] = FName then"

as

Code: [Select]

if FParent.FNames[i] = FName thenie without 'escaping' the i

[GaborBoros]

Hi Tony,

First of all Thank You! for your IBX work and support.

1) Ok. Now I see the Breaks is evil for general use. But fit my needs.  (Only use IBSQL for INSERT and UPDATE with unique parameter names.)

2) My app convert text log files to a database. With oirginal 1.0.5 the speed is 671 parsed log entry/sec. After changed "FParent.FNames" only(, without Breaks and ParamByNameEx) the speed is 764 ple/sec (~13 percent more performance than original).

3) Prepare is not my idea. With ParamByName the next chain is executed and TIBSQL.GetSQLParams contains the Prepare:
TIBSQL.ParamByName -> TIBSQL.GetSQLParams -> TIBXSQLDA.ByName -> TIBXSQLDA.GetXSQLVARByName -> TIBXSQLDA.GetXSQLVAR -> TIBXSQLVAR.SetAsVariant

4) I switched from UIB and want more speed because IBX's INSERT performance with IBSQL is very low for me.
671 ple/sec with original 1.0.5, 902 ple/sec with all my modifications, and 1038 ple/sec with UIB.
I don't understand why IBX is so slow.

5)Other things.

What do you think about varWord, varShortInt and varInt64 in TIBXSQLVAR.SetAsVariant? Without them cannot set parameter value from Word, Byte and Int64 variable.

In TIBQuery.SetParams ftLargeInt is commented out. Without it cannot set parameter value from Int64 variable.

The automatically retry thing is evil in TIBSQL.ExecQuery ("Sometimes a prepared stored procedure appears to get off sync...").
Because if an INSERT INTO RETURNING contains a NEXT VALUE FOR and a UNIQUE or other constraint fired the generator value stepped twice.
Replace

Code: [Select]

      if (fetch_res <> 0) and (fetch_res <> isc_deadlock) then
      begin
         { Sometimes a prepared stored procedure appears to get
           off sync on the server ....This code is meant to try
           to work around the problem simply by "retrying". This
           need to be reproduced and fixed.
         }
        isc_dsql_prepare(StatusVector, TRHandle, @FHandle, 0,
                         PChar(FProcessedSQL.Text), 1, nil);
        Call(isc_dsql_execute2(StatusVector,
                            TRHandle,
                            @FHandle,
                            Database.SQLDialect,
                            FSQLParams.AsXSQLDA,
                            FSQLRecord.AsXSQLDA), True);
      end; with

Code: [Select]

if (fetch_res <> 0) then IBDataBaseError; works for me.

You started the work with original sources? If yes why not with 4.62 from SourceForge?

Gabor

[GaborBoros]

With the direct lookup of the parameter name, you have missed out a call to "FormatIdentifierValue". This is an important omission as this function reformats the placeholder name according to the current SQL Dialect. Your "ParamByNameEx" should only work when the placeholder names you use are not reformatted by "FormatIdentifierValue". This is probably not a good idea.

Hi Tony and All,

If remove FormatIdentifierValue from TIBXSQLDA.GetXSQLVARByName the result is ~9 percent performance gain. I don't understand why FormatIdentifierValue call needed? Please provide an example. Anyone?

Gabor

[BigChimp]

Going by Tony's reply I would suspect that you should try with a Dialect 1 database (instead of the more common Dialect 3):

With the direct lookup of the parameter name, you have missed out a call to "FormatIdentifierValue". This is an important omission as this function reformats the placeholder name according to the current SQL Dialect. Your "ParamByNameEx" should only work when the placeholder names you use are not reformatted by "FormatIdentifierValue". This is probably not a good idea.

[GaborBoros]

Going by Tony's reply I would suspect that you should try with a Dialect 1 database (instead of the more common Dialect 3):

With the direct lookup of the parameter name, you have missed out a call to "FormatIdentifierValue". This is an important omission as this function reformats the placeholder name according to the current SQL Dialect. Your "ParamByNameEx" should only work when the placeholder names you use are not reformatted by "FormatIdentifierValue". This is probably not a good idea.

With Dialect 1 only an UpperCase executed. I don't understand why FormatIdentifierValue exist. Please provide a simple example. Sorry if I am stupid.

Code: [Select]

function FormatIdentifierValue(Dialect: Integer; Value: String): String;
begin
  Value := Trim(Value);
  if Dialect = 1 then
    Value := AnsiUpperCase(Value) 
  else
  begin
    if (Value <> '') and (Value[1] = '"') then
    begin
      Delete(Value, 1, 1);
      Delete(Value, Length(Value), 1);
      Value := StringReplace (Value, '""', '"', [rfReplaceAll]);
    end
    else
      Value := AnsiUpperCase(Value);
  end;
  Result := Value;
end;

[tonyw]

Gabor,

You are raising many questiions and I am not sure that I can deal adequately with all of them in a single post - but here goes.

1. Performance.

The problem that you are having is due to the look up time looking up parameters by name including search for duplicate parameter names. By using positional parameters you should be able to get a much faster response. However, looking at IBX, it will still search for duplicate parameters by name, even when the parameter is selected by position. This is probably wrong and unnecessary. So even if you select the parameter by position, the setter will still execute the

Code: [Select]

for i := 0 to FParent.FCount - 1 do
    if FParent.FNames[i] = FName then
    begin
that is giving you the problem.

However, IBX allows you to use either PSQL style placeholders (e,g, :Param) or DSQL placeholder i.e. a single ? (See TIBSQL.PreprocessSQL and the GenerateParamNames property). In the latter case there can never be duplicate placeholder names and the search is not required. I would thus propose that the best fix for your problem is for the setter to recognise this case and to skip the search. You should then see even better performance as the

Code: [Select]

FParent.FNames[i] = FName is never called in such a case.

It should not be too difficult to achieve this by passing "GenerateParamNames" as a new parameter to  TIBXSQLDA.Initialize and then setting a boolean for each TIBXSQLVAR with a generated name.

2. "I don't understand why FormatIdentifierValue call needed? "

A good question. It should not be needed for placeholder names - it really only applies to field names - case sensitivity is the only issue and not database dialect.

The underlying problem is structural. IBX is old code and this bit probably dates back to the Free IB Components of the 1990s. A criticism that I have is that while TIBXSQLDA and TIBXSQLVAR faithfully encapsulate the Firebird XSQLDA and XSQLVAR, the Firebird structures are dual purpose and are used differently for input and output columns. IMHO, it would have been much better to have had a base class for each and then separate derived classes encapsulating their use for input and output respectively. The code would be much clearer and should avoid problems like the use of FormatIdentifierValue.

On the other hand, it works and is robust and I am not sure that I want to go through the pain of retesting it again to the same level of reliability.

3. "You started the work with original sources? If yes why not with 4.62 from SourceForge?"

At the time I was more concerned with making sure that I did not get into a software licensing problem. I based IBX for Lazarus on the same release as the original FIrebird source. It came with all the licensing artifacts and I could be sure there would be no problem. The sourceforge 2.62 is still very old code _and_ there is no licence file with it - even though each file still has an appropriate header. I was happy with my baseline version and stuck with it, rather than take a risk on the licensing.

4. "What do you think about varWord, varShortInt and varInt64 in TIBXSQLVAR.SetAsVariant? Without them cannot set parameter value from Word, Byte and Int64 variable."

A good point. These should be supported.

5. "In TIBQuery.SetParams ftLargeInt is commented out. Without it cannot set parameter value from Int64 variable."

There was probably a good reason at the time - fpc 2.4.2 limitation perhaps? Needs review, but it may now be possible to uncomment this code.

6 "The automatically retry thing is evil in TIBSQL.ExecQuery"

I have no idea where this came from and would tend to agree. All you need to do is to change "False" to "True" in the first call to isc_dsql_execute2 and delete the rest.

Tony

[tonyw]

A new version, IBX for Lazarus 1.1 is now available from

http://www.mwasoftware.co.uk/ibx

See announcement in the 3rd party forum for the change log.

Many thanks to Gabor for pointing out the performance issues and for testing updates. This kicked me into cleaning up this part of the code and rationalising the way that IBX handles column names with spaces and special characters in them.

[GaborBoros]

Hi All,

I wrote 30 percent in the first post. Tony found more 20 percent and backward compatibility also. With 1.1 my application INSERT performance increased by 50 percent (with disable UseCaseSensitiveParamName and set UniqueParamNames to True)

Thank You very much Tony! 

Gabor