Recent

Author Topic: [CLOSED] Improvements in rtl/objpas/sysutils/fina.inc  (Read 1141 times)

lagprogramming

  • Sr. Member
  • ****
  • Posts: 378
[CLOSED] Improvements in rtl/objpas/sysutils/fina.inc
« on: May 25, 2023, 11:43:53 am »
rtl/objpas/sysutils/fina.inc has the folowing routines:
function ChangeFileExt(const FileName, Extension: PathStr): PathStr;
function ExtractFilePath(const FileName: PathStr): PathStr;
function ExtractFileDir(const FileName: PathStr): PathStr;
function ExtractFileName(const FileName: PathStr): PathStr;
function ExtractFileExt(const FileName:PathStr):PathStr;

Switching the "I" and "EndSep" assignment lines makes the compiler produce better code. You can verify by looking at the generated assembly file.
I'm talking about the lines like:
Code: Pascal  [Select][+][-]
  1.   I := Length(FileName);
  2.   EndSep:=AllowDirectorySeparators+AllowDriveSeparators;
 
The following patch modifies all the above functions.
Code: Pascal  [Select][+][-]
  1. diff --git a/rtl/objpas/sysutils/fina.inc b/rtl/objpas/sysutils/fina.inc
  2. index e817d60e6a..95217a026b 100644
  3. --- a/rtl/objpas/sysutils/fina.inc
  4. +++ b/rtl/objpas/sysutils/fina.inc
  5. @@ -21,8 +21,8 @@ function ChangeFileExt(const FileName, Extension: PathStr): PathStr;
  6.    SOF : Boolean;
  7.    
  8.  begin
  9. -  i := Length(FileName);
  10.    EndSep:=AllowDirectorySeparators+AllowDriveSeparators+[ExtensionSeparator];
  11. +  i := Length(FileName);
  12.    while (I > 0) and not(FileName[I] in EndSep) do
  13.      Dec(I);
  14.    if (I = 0) or (FileName[I] <> ExtensionSeparator) then
  15. @@ -41,8 +41,8 @@ function ExtractFilePath(const FileName: PathStr): PathStr;
  16.    i : longint;
  17.    EndSep : Set of Char;
  18.  begin
  19. -  i := Length(FileName);
  20.    EndSep:=AllowDirectorySeparators+AllowDriveSeparators;
  21. +  i := Length(FileName);
  22.    while (i > 0) and not CharInSet(FileName[i],EndSep) do
  23.      Dec(i);
  24.    If I>0 then
  25. @@ -56,8 +56,8 @@ function ExtractFileDir(const FileName: PathStr): PathStr;
  26.    i : longint;
  27.    EndSep : Set of Char;
  28.  begin
  29. -  I := Length(FileName);
  30.    EndSep:=AllowDirectorySeparators+AllowDriveSeparators;
  31. +  I := Length(FileName);
  32.    while (I > 0) and not CharInSet(FileName[I],EndSep) do
  33.      Dec(I);
  34.    if (I > 1) and CharInSet(FileName[I],AllowDirectorySeparators) and
  35. @@ -102,8 +102,8 @@ function ExtractFileName(const FileName: PathStr): PathStr;
  36.    i : longint;
  37.    EndSep : Set of Char;
  38.  begin
  39. -  I := Length(FileName);
  40.    EndSep:=AllowDirectorySeparators+AllowDriveSeparators;
  41. +  I := Length(FileName);
  42.    while (I > 0) and not CharInSet(FileName[I],EndSep) do
  43.      Dec(I);
  44.    Result := Copy(FileName, I + 1, MaxInt);
  45. @@ -117,8 +117,8 @@ function ExtractFileExt(const FileName: PathStr): PathStr;
  46.    
  47.  begin
  48.    Result:='';
  49. -  I := Length(FileName);
  50.    EndSep:=AllowDirectorySeparators+AllowDriveSeparators+[ExtensionSeparator];
  51. +  I := Length(FileName);
  52.    while (I > 0) and not CharInSet(FileName[I],EndSep) do
  53.      Dec(I);
  54.    if (I > 0) and (FileName[I] = ExtensionSeparator) then
« Last Edit: June 04, 2023, 09:00:04 am by lagprogramming »

Thaddy

  • Hero Member
  • *****
  • Posts: 13213
Re: Improvements in rtl/objpas/sysutils/fina.inc
« Reply #1 on: May 25, 2023, 12:10:46 pm »
Why would you slow down the code with this patch?
I actually get compliments for being rude... (well, Dutch, but that is the same)

marcov

  • Administrator
  • Hero Member
  • *
  • Posts: 10944
  • FPC developer.
Re: Improvements in rtl/objpas/sysutils/fina.inc
« Reply #2 on: May 25, 2023, 12:37:05 pm »
The window that the variable is live shortens, increasing the chances that it can be registered. Perhaps the compiler should be able to do this automatically though :-)

lagprogramming

  • Sr. Member
  • ****
  • Posts: 378
Re: Improvements in rtl/objpas/sysutils/fina.inc
« Reply #3 on: May 25, 2023, 04:33:07 pm »
AllowDirectorySeparators, AllowDriveSeparators and ExtensionSeparator are all constants, but I couldn't turn EndSep from a variable to a constant. Turning EndSep variables into constants will make the patch obsolete.
I mean I've tried adding
Code: Pascal  [Select][+][-]
  1. const
  2.   EndSep : Set of Char = AllowDirectorySeparators+AllowDriveSeparators+[ExtensionSeparator];
in order to help the compiler produce better code but the compiler refused to build the modified file.
It might be something trivial to make the compiler compute the EndSep values at compile time, I don't know, or maybe it should already compute the EndSep values at compile time and there is a bug.  :-\

PascalDragon

  • Hero Member
  • *****
  • Posts: 5214
  • Compiler Developer
Re: Improvements in rtl/objpas/sysutils/fina.inc
« Reply #4 on: May 26, 2023, 10:26:56 pm »
AllowDirectorySeparators, AllowDriveSeparators and ExtensionSeparator are all constants, but I couldn't turn EndSep from a variable to a constant. Turning EndSep variables into constants will make the patch obsolete.
I mean I've tried adding
Code: Pascal  [Select][+][-]
  1. const
  2.   EndSep : Set of Char = AllowDirectorySeparators+AllowDriveSeparators+[ExtensionSeparator];
in order to help the compiler produce better code but the compiler refused to build the modified file.
It might be something trivial to make the compiler compute the EndSep values at compile time, I don't know, or maybe it should already compute the EndSep values at compile time and there is a bug.  :-\

AllowDirectorySeperators and AllowDriveSeparators are typed constants. Typed constants can not be used as an initializer for constants (typed or untyped) or variables, thus you simply can't declare a EndSep constant.

lagprogramming

  • Sr. Member
  • ****
  • Posts: 378
Re: Improvements in rtl/objpas/sysutils/fina.inc
« Reply #5 on: May 27, 2023, 04:06:20 pm »
AllowDirectorySeperators and AllowDriveSeparators are typed constants. Typed constants can not be used as an initializer for constants (typed or untyped) or variables, thus you simply can't declare a EndSep constant.

Trying to use conditional compilation fails too
Code: Pascal  [Select][+][-]
  1. {$if AllowDirectorySeparators=['\','/']}
  2. //Code
  3. {$endif}
So, either we wait for the compiler to be modified in order to be able to compute at compile time these constant sets, either the local EndSep variables would be replaced by global unit variables that would be computed at unit initialization(this should work, right?). I have doubts the last one would be accepted in the official sources.
If you have a long list of files that you should sort using such routines, it will take a lot of time.
I don't know which solution should be applied because I have no idea how much work has to be done in order to make the compiler optimize such code. Computing the values at compile time would be my first choice.

PascalDragon

  • Hero Member
  • *****
  • Posts: 5214
  • Compiler Developer
Re: Improvements in rtl/objpas/sysutils/fina.inc
« Reply #6 on: May 27, 2023, 06:35:26 pm »
AllowDirectorySeperators and AllowDriveSeparators are typed constants. Typed constants can not be used as an initializer for constants (typed or untyped) or variables, thus you simply can't declare a EndSep constant.

Trying to use conditional compilation fails too
Code: Pascal  [Select][+][-]
  1. {$if AllowDirectorySeparators=['\','/']}
  2. //Code
  3. {$endif}

Same reason. Typed constants can not be used in compiler directives.

So, either we wait for the compiler to be modified in order to be able to compute at compile time these constant sets,

In principle one could already declare these constants as untyped set constants (I don't know right whether there is a reason that would prohibit changing this as typed and untyped constants behave different in some circumstances - e.g. you can't take the address of an untyped constant):

Code: Pascal  [Select][+][-]
  1. const
  2.   AllowDirectorySeparators = ['/', '\'];

But the compiler won't get the ability to calculate with typed constants at compile time.

either the local EndSep variables would be replaced by global unit variables that would be computed at unit initialization(this should work, right?). I have doubts the last one would be accepted in the official sources.

Well, you could use a local writable constant (which is essentially a static variable) - though I don't say that we'd accept that:

Code: Pascal  [Select][+][-]
  1. procedure SomeProc;
  2. {$push}{$J+}
  3. const
  4.   EndSep: set of Char = [];
  5. {$pop}
  6. begin
  7.   if EndSep = [] then
  8.     EndSep := AllowDirectorySeparators + AllowDriveSeparators;
  9.   {... }
  10. end;

From the second call of SomeProc on the value of EndSep will no longer be empty. This won't be threadsafe however (it could be fixed by using something like InterlockedExchange however).

lagprogramming

  • Sr. Member
  • ****
  • Posts: 378
Re: Improvements in rtl/objpas/sysutils/fina.inc
« Reply #7 on: May 30, 2023, 09:01:19 am »
Why would you slow down the code with this patch?
When the CPU target was x86_64, for each modified routine I've noticed one less push/pop pair instructions.

 

TinyPortal © 2005-2018