7.2 testers-thread

[EgonHugeist]

Hmpf,

note the IdentiferConv (short IC ) makes loads of roundtrips to dermine Lower/Upper/Mixed cases. This could be optimes to one rountrip if we retrieve an enum.

Accordingly the bechmarks .. Attached is next tiny RowAccessor simplification which does not use virtual function nor the 'Obj Is TZRawRowAccessor' syntax. i must admit i've no AQTime. It's based to your findings of using a simple Bool to know Raw vs. Unicode storage. Could you test differences, please. If it's faster we'll commit the patch. It reduces the lines again a loads.

[Fr0sT]

note the IdentiferConv (short IC ) makes loads of roundtrips to dermine Lower/Upper/Mixed cases. This could be optimes to one rountrip if we retrieve an enum.

You've guessed right my thoughts
I even implemented this change (just a test class I extracted from identconv, and I haven't tested it with various inputs)

Code: Select all

type
  TCase = (caseNo, caseSpec, caseLo, caseUp);

function Tconv.GetCase(const Value: string): TCase;
var
  P: PChar;
  UpCnt, LoCnt: Integer;
begin
  Result := caseNo;
  if Value = '' then Exit;
  P := Pointer(Value);
  if CharInSet(P^, ['0'..'9']) then
  begin
    Result := caseSpec;
    Exit;
  end;
  UpCnt := 0; LoCnt := 0;
  while P^<> #0 do begin
    case P^ of
      'A'..'Z':
          Inc(UpCnt);
      'a'..'z':
          Inc(LoCnt);
      '0'..'9','_':
        ;
      else
        Exit(caseSpec);
    end;
    Inc(P);
  end;
  if (UpCnt > 0) and (LoCnt = 0) then
    Result := caseUp
  else
  if (UpCnt = 0) and (LoCnt > 0) then
    Result := caseLo
  else
    Result := caseNo;
end;

function Tconv.IsCaseSensitive_2(const Value: string): Boolean;
var valcase: tcase;
begin
  if Value = '' then
    Exit(False);
    valcase := GetCase(value);
  case valcase of
    caseNo: Result := not False; // Metadata.GetDatabaseInfo... must be here
    caseSpec: Result := True;
    caseLo: Result := True;  // Metadata.GetDatabaseInfo... must be here
    caseUp: Result := False;  // Metadata.GetDatabaseInfo... must be here
  end;
...
end;

But it gave only 860 ops/sec vs initial 790. What is really damn slow is

Code: Select all

    Keywords := ',' + AnsiSQLKeywords + ','
      + LowerCase(TDBInfo.GetSQLKeywords) + ',';
    Result := ZFastCode.Pos(',' + LowerCase(Value) + ',', Keywords) > 0;

I optimized this search with pre-constructed array of strings and trivial for-loop search which gave speedup of 2760 ops/sec and 3270 with GetCase. Surprisingly all my further tricks with binary search on sorted array, hash map searches etc were beaten by this loop
Considering that IC is mainly created once, we could get real benefit from this optimization. Only two remaining places are calls to DefineKeyFields routine from T*Dataset methods. Maybe we could save IC as a field in SetConnection?

[EgonHugeist]

Hi FrOsT,

could you please rollback the simplification of

Code: Select all

   { Looks by field index. }
-  {$IFDEF GENERIC_INDEX}
-  if (ColumnIndex >= 0) and (ColumnIndex < FFields.Count) then
-  {$ELSE}
-  if (ColumnIndex > 0) and (ColumnIndex <= FFields.Count) then
-  {$ENDIF}
+  if (ColumnIndex >= FirstDbcIndex) and (ColumnIndex <= FFields.Count {$IFDEF GENERIC_INDEX}-1{$ENDIF}) then

we've a circulare reference from ZParseSQL to ZDbc now. Loads of users will have compilation issues since ZDbcInfs is included.

Accordingly the IC stuff:

Based on your Proposal i've attached another patch which will compile on older delphis too. Could you have a look at? I don't use the loop of:

I optimized this search with pre-constructed array of strings and trivial for-loop search which gave speedup of 2760 ops/sec and 3270 with GetCase. Surprisingly all my further tricks with binary search on sorted array, hash map searches etc were beaten by this loop

Does it meen you prebuild a TStringDynArray oslt? Thought Pos() schould be faster? Is it slower if the String is precomposed like in my example?

[Fr0sT]

Hi Michael,

could you please rollback the simplification

Yep, did it just now. Having paths declared in IDE settings makes this kind of bugs hard to detect...

Does it meen you prebuild a TStringDynArray oslt? Thought Pos() schould be faster? Is it slower if the String is precomposed like in my example?

Yes, I added array of string filled with keywords and it gave real speedup over Pos even if the string of keywords is preconstructed. I guess this happens because most of comparisons of separate strings exit early at the 1st symbol while Pos gives first positive for every keyword in the string (it detects ",") and goes to nested loop.
My benchmark reports 700 / 2720 / 1170 ops/sec for Is*Case + Pos in constructed string (old variant) / Is*Case + find in array / GetCase + Pos in prebuilt string.
Btw, there's a catch in prebuilding something in IC's constructor. In TZAbstractDatabaseMetadata.Create FIC is created before FDatabaseInfo so Metadata.GetDatabaseInfo.GetSQLKeywords in TZDefaultIdentifierConvertor.Create will raise AV. To fix this issue I thought of two options:
- make FIC creation happen after FDatabaseInfo creation (the easiest)
- add lazy initialization to TZDefaultIdentifierConvertor itself: flag FInited, internal method Init and check "if not FInited then Init".

EDIT

Regarding your edit:
- First "Inc(LoCnt);" should be "Inc(UpCnt);"
- Just FYI: CharInSet is good but only with inlining enabled, otherwise direct "case" wins.
- Generally every conditional branching slows down the execution, that's why I used counters instead of plain flags and checks on each symbol (that was my first realization). In benchmark my variant is slightly better but it's all about 100 msecs of difference on 25M iterations. OTOH, this happens for values fully in one case. With value in mixed case your variant with condition checks gives big speedup.
So I tend to accept realization with conditions. Surprisingly the slightly simplified modification that uses Boolean flags:

Code: Select all

function Tconv.GetCase2(const Value: String): TCase;
var
  P: PChar;
  UpCase, LoCase: Boolean;
begin
  Result := caseNo;
  if Value = '' then Exit;
  P := Pointer(Value);
  case P^ of //don't use damn slow charInSet
    '0'..'9': begin
      Result := caseSpec;
      Exit;
    end;
  end;
  UpCase := False; LoCase := False;
  while P^<> #0 do begin
    case P^ of
      'A'..'Z':
        if LoCase then begin //stop loop
          Result := caseSpec;
          Exit;
        end else
          UpCase := True;
      'a'..'z':
        if UpCase then begin //stop loop
          Result := caseSpec;
          Exit;
        end else
          LoCase := True;
      '0'..'9','_':
        ;
      else begin //stop loop
        Result := caseSpec;
        Exit;
      end;
    end;
    Inc(P);
  end;
  if UpCase then
    Result := caseUp
  else if LoCase then
    Result := caseLo
  else //this could happen only if table starts with '_' and possible numbers follow
    Result := caseNo;
end;

happens to be slower o__O Contemporary CPUs seem to do some magic a human couldn't understand.

[EgonHugeist]

Hi FrOsT,

Yep, did it just now. Having paths declared in IDE settings makes this kind of bugs hard to detect...

Thanks for that new solution is pretty nice!

My benchmark reports 700 / 2720 / 1170 ops/sec for Is*Case + Pos in constructed string (old variant) / Is*Case + find in array / GetCase + Pos in prebuilt string.

Awesome! just add these findings as comment!

TZDefaultIdentifierConvertor.Create

which means the converter is assigned somwhere before the Connection is opened?

That's why IZDatabaseMetatdata.GetIdentifierConvertor allways retrieves a new object. Advantages:
1. You don't need to care if connection is open.
2. Thread-Safe.
3. if connection and it's protocol changes you'll retrieve the expected interface for the job.

The disadvantages is just because of the splitted DynArray.

How to Resolve:

Code: Select all

lazy initialization to TZDefaultIdentifierConvertor itself: flag FInited, internal method Init and check "if not FInited then Init"

might be an solutions..

What about an overload for GetSQLKeywords like

Code: Select all

procedure IZDatabaseMetadat.GetSQLKeywords(out: KeyWords: TStringDynArray); overload;
or 
function ZDatabaseMetadat.GetSQLKeywordsSplitted: TStringDynArray;

So the dynarray initialization happens in TZAbstractDatabaseInfo only and each Interface uses them. And you don't need to care about other code..

What do you think?

[Fr0sT]

Michael,
I also added some words to the previous post

Thanks for that new solution is pretty nice!

Cool

which means the converter is assigned somwhere before the Connection is opened?

I'm not sure I got what you mean... Metadata is created with an opened connection, AFAIU. But DBInfo is created after IdConv so it couldn't be used in IdConv's constructor.

That's why IZDatabaseMetatdata.GetIdentifierConvertor allways retrieves a new object. Advantages:
1. You don't need to care if connection is open.
2. Thread-Safe.
3. if connection and it's protocol changes you'll retrieve the expected interface for the job.

Well, I came to same conclusion. But:
1. If Metadata exists at all, it has the same IC. Closed or opened connection doesn't affect IC. But the case should be checked when a connection was used with one driver then closed and used with another one.
2. IC's methods neither change a thing nor depend on potentially changeable variables so they could be safely used in multithreaded env. Anyway multithreaded use of DB-aware components is VERY bad practice
3. Yep, this case we must check.

What about an overload for GetSQLKeywords like

procedure IZDatabaseMetadat.GetSQLKeywords(out: KeyWords: TStringDynArray); overload;
or
function ZDatabaseMetadat.GetSQLKeywordsSplitted: TStringDynArray;
So the dynarray initialization happens in TZAbstractDatabaseInfo only and each Interface uses them. And you don't need to care about other code..

Pretty good! Though we have constant part of ANSI-standartd keywords as well so it will require two array searches. Or we could move these keywords to base metadata class and construct the array with them already included.

[EgonHugeist]

FrOsT,

If Metadata exists at all, it has the same IC.

I do not understand. Metadata constructs the IC. Except this interface is localized somewhere.
In addition i'm trying to say with my proposel we don't need this Object "cached".

IC's methods neither change a thing nor depend on potentially changeable variables so they could be safely used in multithreaded env.

True but i can't see what happens in future..

Anyway multithreaded use of DB-aware components is VERY bad practiceBoth is true.

We had bugreports already. And they use the TComponents. I use Zeos in mutithreading env. too, but plain dbc only.

Pretty good! Though we have constant part of ANSI-standartd keywords as well so it will require two array searches. Or we could move these keywords to base metadata class and construct the array with them already included.

For me it's plain wrong that the IC contains consts! All these keywords should be returned by GetSQLKeyWords.

So as you suggest the TZAbstractDatabaseMetadata should return this minimal set of keywords and it's descendands should extend them.
If this is done the StringDynArray could be simple made by something like

Code: Select all

begin
  if Pointer(FKeyWordArray) = nil then
    FKeyWordArray := Split(GetSQLKeyWords);
  ResultOrOutParam := FKeyWordArray;
end;

[Fr0sT]

Michael, nevermind with keywords array totally hosted by Metadata all discussion regarding creation/caching objects loses its sense. The solution is brilliant!

I use Zeos in mutithreading env. too, but plain dbc only.

Using in MT is OK as long as you keep each component in its own thread. Driver client libs are mainly not thread-safe themselves. Bothering about thread-safe of classes built upon these libs is waste of time.

For me it's plain wrong that the IC contains consts! All these keywords should be returned by GetSQLKeyWords.

Agree!

So as you suggest the TZAbstractDatabaseMetadata should return this minimal set of keywords and it's descendands should extend them.
If this is done the StringDynArray could be simple made by something like

Code: Select all

begin
  if Pointer(FKeyWordArray) = nil then
    FKeyWordArray := Split(GetSQLKeyWords);
  ResultOrOutParam := FKeyWordArray;
end;

Yes, that's pretty good. Would you make this change or it'll wait? I'm away from IDE for a week. Of course I can write some code in notepad but not compile it

Sorry I haven't got a time to check your RowAccessor.patch. I'll try to examine it soon

[DreamDancer]

Hallo

SVN 4150
bringt folgende Fehlermeldungen (in Lazarus 1.8.0 Win32+Win64)

Kompiliere Package zdbc 7.2: Exit code 1, Fehler: 3
ZDbcMetadata.pas(1017,36) Error: Variable identifier expected
ZDbcMetadata.pas(1018,25) Error: Variable identifier expected
ZDbcMetadata.pas(1020,25) Error: Variable identifier expected

[miab3]

@ Fr0sT, @EgonHugeist, @mdaems, @Jan, All

ZEOS 7.3.x svn 4156:
http://sourceforge.net/p/zeoslib/code-0 ... sting-7.3/

compile and run on:

- Lazarus 1.8.0-Win32 (Windows 10-32),
- Delphi 2007-Win32 (Windows 10-64),
- Delphi 10 Seattle-Win32/Win64 (Windows 10-64).

Michal

[Prometeus]

Hello,

For the latest releases, I am able to compile for Delphi Tokyo 10.2.2 only if I use the package for Delphi Berlin. The package for Delphi Tokyo keep asking to include 'core' on the project but always give an error. The package for Delphi Berlin 10.1 works ok on Delphi Tokyo 10.2.2, however.

Regards

[miab3]

@ Fr0sT, @EgonHugeist, @mdaems, @Jan, All

ZEOS 7.3.x svn 4162:
http://sourceforge.net/p/zeoslib/code-0 ... sting-7.3/

compile and run on:

- Lazarus 1.8.0-Win32 (Windows 10-32),
- Delphi 2007-Win32 (Windows 10-64),
- Delphi 10 Seattle-Win32/Win64 (Windows 10-64).

Michal

[EgonHugeist]

Thanks mib3.

@Prometeus

Fixes? Something we can do? Nobody of current team has a XE 10.2. So it would be nice if you can help..

@Frost,

most things are done. Note behavior changes haven't been made yet. I'll continue implement a test for the tkszeos report...

[Prometeus]

@Prometeus

Fixes? Something we can do? Nobody of current team has a XE 10.2. So it would be nice if you can help..

@EgonHugeist, the attached package is now compiling and installing with no issues for Delphi Tokyo 10.2.2

Regards

[DreamDancer]

Compiling SVN 4179 shows this error:

ZDbcAdoMetadata.pas(692,9) Error: Identifier not found "VariantClear"

Lazarus 1.8 Win7 64