[bug_fixed] Insert "const" for ref-counted paramet

[AHUser]

The Delphi Compiler will generate faster code if you use the "const" modifier in parameters of the types "string", "variant" and dynamic/open arrays. Also records with such fields can be optimized with a simple "const". (best example: TZVariant)

Example:

Code: Select all

function ArrayToStrings(Value: array of string): TStrings;
var
  I: Integer;
begin
  Result := TStringList.Create;
  for I := Low(Value) to High(Value) do
    Result.Add(Value[I]);
end;

This generates:

Code: Select all

FrmMain.pas.209: begin
00629748 55               push ebp
00629749 8BEC             mov ebp,esp
0062974B 83C4F8           add esp,-$08
0062974E 53               push ebx
0062974F 56               push esi
00629750 57               push edi
00629751 8BCA             mov ecx,edx
00629753 85C9             test ecx,ecx
00629755 7807             js $0062975e
00629757 8B1C88           mov ebx,[eax+ecx*4]
0062975A 49               dec ecx
0062975B 53               push ebx
0062975C 79F9             jns $00629757
0062975E 8BC4             mov eax,esp
00629760 8955F8           mov [ebp-$08],edx
00629763 8945FC           mov [ebp-$04],eax
00629766 8B4DF8           mov ecx,[ebp-$08]
00629769 41               inc ecx
0062976A 8B45FC           mov eax,[ebp-$04]
0062976D 8B15F8104000     mov edx,[$004010f8]
00629773 E8E4C5DDFF       call @AddRefArray
00629778 33C0             xor eax,eax
0062977A 55               push ebp
0062977B 68CE976200       push $006297ce
00629780 64FF30           push dword ptr fs:[eax]
00629783 648920           mov fs:[eax],esp
FrmMain.pas.210: Result := TStringList.Create;
00629786 B201             mov dl,$01
00629788 A10CE74100       mov eax,[$0041e70c]
0062978D E88AA6DDFF       call TObject.Create
00629792 8BF8             mov edi,eax
FrmMain.pas.211: for I := Low(Value) to High(Value) do
00629794 8B75F8           mov esi,[ebp-$08]
00629797 85F6             test esi,esi
00629799 7C13             jl $006297ae
0062979B 46               inc esi
0062979C 8B5DFC           mov ebx,[ebp-$04]
FrmMain.pas.212: Result.Add(Value[I]);
0062979F 8B13             mov edx,[ebx]
006297A1 8BC7             mov eax,edi
006297A3 8B08             mov ecx,[eax]
006297A5 FF5138           call dword ptr [ecx+$38]
006297A8 83C304           add ebx,$04
FrmMain.pas.211: for I := Low(Value) to High(Value) do
006297AB 4E               dec esi
006297AC 75F1             jnz $0062979f
FrmMain.pas.213: end;
006297AE 33C0             xor eax,eax
006297B0 5A               pop edx
006297B1 59               pop ecx
006297B2 59               pop ecx
006297B3 648910           mov fs:[eax],edx
006297B6 68D5976200       push $006297d5
006297BB 8B45FC           mov eax,[ebp-$04]
006297BE 8B4DF8           mov ecx,[ebp-$08]
006297C1 41               inc ecx
006297C2 8B15F8104000     mov edx,[$004010f8]
006297C8 E84FC4DDFF       call @FinalizeArray
006297CD C3               ret 
006297CE E94DAEDDFF       jmp @HandleFinally
006297D3 EBE6             jmp $006297bb
006297D5 8BC7             mov eax,edi
006297D7 8B7DEC           mov edi,[ebp-$14]
006297DA 8B75F0           mov esi,[ebp-$10]
006297DD 8B5DF4           mov ebx,[ebp-$0c]
006297E0 8BE5             mov esp,ebp
006297E2 5D               pop ebp
006297E3 C3               ret 

And now with "const"

Code: Select all

function ArrayToStrings({>>} const {<<}Value: array of string): TStrings;
var
  I: Integer;
begin
  Result := TStringList.Create;
  for I := Low(Value) to High(Value) do
    Result.Add(Value[I]);
end;

Code: Select all

FrmMain.pas.209: begin
00629748 53               push ebx
00629749 56               push esi
0062974A 57               push edi
0062974B 8BF2             mov esi,edx
0062974D 8BD8             mov ebx,eax
FrmMain.pas.210: Result := TStringList.Create;
0062974F B201             mov dl,$01
00629751 A10CE74100       mov eax,[$0041e70c]
00629756 E8C1A6DDFF       call TObject.Create
0062975B 8BF8             mov edi,eax
FrmMain.pas.211: for I := Low(Value) to High(Value) do
0062975D 85F6             test esi,esi
0062975F 7C10             jl $00629771
00629761 46               inc esi
FrmMain.pas.212: Result.Add(Value[I]);
00629762 8B13             mov edx,[ebx]
00629764 8BC7             mov eax,edi
00629766 8B08             mov ecx,[eax]
00629768 FF5138           call dword ptr [ecx+$38]
0062976B 83C304           add ebx,$04
FrmMain.pas.211: for I := Low(Value) to High(Value) do
0062976E 4E               dec esi
0062976F 75F1             jnz $00629762
FrmMain.pas.213: end;
00629771 8BC7             mov eax,edi
00629773 5F               pop edi
00629774 5E               pop esi
00629775 5B               pop ebx
00629776 C3               ret 

You see the difference? So using "const" will improve the speed.


I could do this for you, but that meight be a very large patch.

[mdaems]

Hi,

I think you're right.

BUT:
* Do the other compilers support it? If not you should make sure it's commented out for the other compilers.
* You may start patching the modules one by one. I would propose you start with the Core units. Negative reactions will come pretty soon if there might go something wrong.
* Please run the test suite before and after doing the patch, to make sure you don't get more errors afterwards. (For me the test runs don't work 100% without failures, but the Core tests do)

I'm the first to test your modifications on D7 and D2006 as soon as they appear, provided you do not modify all at once. If you want to modify a database unit : I use Mysql5.

Succes,

Mark

[AHUser]

Here is the first patch (against SVN revision 32). It does not patch the ZDbc units at the moment (because that is a lot more work than I needed the other packages).


Not tested against FPC. Maybe FPC does not like the

property Bla[const Index: string]: TBlabla read...;

[mdaems]

Hi, AHUser.

I've just applied your patch. It doesn't seem to give trouble in D7. The testsuite didn't give more problems than before either. So, I would advise others to give it a try on other platforms.

Firmos, would you do the integration?

Mark

[AHUser]

The code is compatible from Delphi 5 to BDS 2006.

And I tested the "property bla[const Index: string]..." with FPC 2.0.0 [2005/05/08]. So this is also no problem.

[mdaems]

Hi AHUser,

I applied your patch to the (new) testing branch in the SVN repository. This branch is the development branch to avoid disturbing the trunk 'production' branch. As a contributer I propose you change to the testing branch.

It would be nice if you could go on modifying the other units, not yet changed. If you can submit a patch to the testing branch, integration should be a matter of hours or at most some days.

Mark

[AHUser]

It would be nice if you could go on modifying the other units, not yet changed.

I'll do it. But I don't know when. Maybe today, monday or sometimes next week.

BTW: There is a CVS directory in the "artwork" directory. I think this one shouldn't be there.

[mdaems]

Hi AHUser,

I removed the files for cvs, but can't manage to remove the directories, themselves. I probably don't know enough about SVN to avoid the message 'Error: Cannot non-recursively commit a directory deletion'.
Maybe later on I will succeed.

Concerning the patch : nothing's burning, so just take your time to do it good.

Mark

[aperger]

All from you:

"..."

I am happy to see you here: Attila Perger

[zippo]

Just for curiosity - how's the speed improvement? Is it measurable?

[mdaems]

Zippo,

Wrote some stupid 'proof' of the speedup, because I was curious as well. Don't tell me it's not accurate. I know. But I'm sure its hows something.
For a call to an 'empty' function (assigning an integer to result) it's about 80 % faster.
Using the example given by AHUser the overall gain is much smaller. Only about 1 %. The other statements in the function take most of the time, so we are only taking 80% of a small partof the function.

My test-project is attached.
Did the check on Lazarus. There the gain is 95%. But the rest of the function doesn't work very fast, so profit is almost invisible.

Mark