Page 33 of 35
Re: 7.2-Beta testers-thread
Posted: 01.02.2018, 18:48
by 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.
Re: 7.2-Beta testers-thread
Posted: 02.02.2018, 09:12
by 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?
Re: 7.2-Beta testers-thread
Posted: 02.02.2018, 12:06
by 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?
Re: 7.2-Beta testers-thread
Posted: 02.02.2018, 14:47
by 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.
Re: 7.2-Beta testers-thread
Posted: 02.02.2018, 15:33
by 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?
Re: 7.2-Beta testers-thread
Posted: 02.02.2018, 16:26
by 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.
Re: 7.2-Beta testers-thread
Posted: 02.02.2018, 17:31
by 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;
Re: 7.2-Beta testers-thread
Posted: 02.02.2018, 21:57
by 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
Re: 7.2-Beta testers-thread
Posted: 06.02.2018, 14:33
by 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
Re: 7.2-Beta testers-thread
Posted: 07.02.2018, 19:45
by 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
Re: 7.2-Beta testers-thread
Posted: 08.02.2018, 04:48
by 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
Re: 7.2-Beta testers-thread
Posted: 09.02.2018, 12:29
by 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
Re: 7.2-Beta testers-thread
Posted: 09.02.2018, 20:26
by 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...
Re: 7.2-Beta testers-thread
Posted: 10.02.2018, 01:15
by Prometeus
EgonHugeist wrote:
@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
Re: 7.2-Beta testers-thread
Posted: 11.02.2018, 13:10
by DreamDancer
Compiling SVN 4179 shows this error:
ZDbcAdoMetadata.pas(692,9) Error: Identifier not found "VariantClear"
Lazarus 1.8 Win7 64