Recent

Author Topic: To optimize RTL, make function PCharToInt  (Read 32499 times)

Alextp

  • Hero Member
  • *****
  • Posts: 1403
    • UVviewsoft
To optimize RTL, make function PCharToInt
« on: July 22, 2021, 09:06:58 am »
As I see per this patch
https://github.com/graemeg/freepascal/commit/71e8160cb755f38cae6ce40db0bd72f968c5a95c
RTL has places where you do StrToInt(Copy(S, n, m)) and StrToIntDef(Copy(...)) and TryStrToInt(Copy(...)).
This can be optimized with a PCharToInt which won't do string allocs.
Code of PCharToInt is simple (using pointer+length).
Agree?
« Last Edit: July 22, 2021, 09:48:12 am by Alextp »

BeniBela

  • Hero Member
  • *****
  • Posts: 817
    • homepage
Re: To optimize RTL, make function PCharToInt
« Reply #1 on: July 22, 2021, 12:28:20 pm »
It is all over the RTL

Not just StrToInt. This goes much much deeper.



Just a quick search:

In inifiles.pp: Trim(Copy(sLine, 1,  j - 1))
In  fphttpclient.pp: LowerCase(Copy(S,1,Length(SetCookie)))=SetCookie
In fphttpclient.pp: ((LowerCase(Copy(HTTPHeaders[Result],1,l)))<>h)
In fpreport.pp: SameText(Copy(BaseName,1,9),'TFPReport')


The proper solution would be to have a new string type SubString or something, which could be used anywhere a string can be used. But it would only refer an existing string, and not require memory allocations.


You could build your own type with a record, but then the problem is that record fields are not kept in registers, so it is slower than a local variable pchar. This needs an improved optimizer

Alextp

  • Hero Member
  • *****
  • Posts: 1403
    • UVviewsoft
Re: To optimize RTL, make function PCharToInt
« Reply #2 on: July 22, 2021, 12:45:43 pm »
These 3 can be opt'ed already:

In  fphttpclient.pp: LowerCase(Copy(S,1,Length(SetCookie)))=SetCookie
In fphttpclient.pp: ((LowerCase(Copy(HTTPHeaders[Result],1,l)))<>h)
In fpreport.pp: SameText(Copy(BaseName,1,9),'TFPReport')

Using StrLComp() and StrLIComp().

jamie

  • Hero Member
  • *****
  • Posts: 4911
Re: To optimize RTL, make function PCharToInt
« Reply #3 on: July 22, 2021, 11:17:27 pm »
Please stop coming up with these badly thought out ideas.

It only aids in adding more useless redundant junk to an already over whelmed RTL..

 Just the naming "PcharToInt" is wrong to start with..

 That means to me is you are coverting a Pointer to a char to an Integer. This of course will only work on 32 bit systems since an integer is 32 bit on most platforms..

 Btw, there is already types for this UintPtr, PtrUint etc..

 Please learn to use what it already in the RTL and if you really think that would be something special for you then please use that in your own code. Don't subject the rest of the community to added bloat of functions that never get used but yet gets compiled in code without any options to remove it..

 Myself I really don't care because I tend to use Delphi lately and it seems to weed out useless code like this better however, I haven't see fpc do very well in removing un-called functions from user code.

 The helpers is a perfect example where people just keep adding on functions that 99% of the coders never need but yet gets linked in your code..

 Is that what you want? That you know everyone's Lazarus app has a piece of your code that never gets used sitting in their app taking up space ?

 really...

The only true wisdom is knowing you know nothing

PascalDragon

  • Hero Member
  • *****
  • Posts: 3384
  • Compiler Developer
Re: To optimize RTL, make function PCharToInt
« Reply #4 on: July 23, 2021, 10:49:31 am »
Please stop coming up with these badly thought out ideas.

Please stop making claims without supporting them with evidence.

If a function is not used in your program then it will be smartlinked out, especially if you did indeed enable the smartlink options. If it is not left out then it is indeed used either directly or indirectly.

Take the following program:

Code: Pascal  [Select][+][-]
  1. program tsmartlink;
  2.  
  3. procedure Test;
  4. begin
  5.   Writeln('Test');
  6. end;
  7.  
  8. procedure TestSub;
  9. begin
  10.   Writeln('TestSub');
  11. end;
  12.  
  13. procedure Test2;
  14. begin
  15.   TestSub;
  16. end;
  17.  
  18. procedure TestSub2;
  19. begin
  20.   Writeln('TestSub2');
  21. end;
  22.  
  23. procedure Test3(aArg: Integer); inline;
  24. begin
  25.   if aArg = 2 then
  26.     TestSub2;
  27. end;
  28.  
  29. begin
  30. {$ifdef USE_TEST}
  31.   Test;
  32. {$endif}
  33. {$ifdef USE_TEST2}
  34.   Test2;
  35. {$endif}
  36. {$ifdef USE_TEST3}
  37.   Test3(1);
  38. {$endif}
  39. end.

If you compile without any of the defines set (just call the compiler manually so that none of Lazarus' options intefere) then you won't find any of the three strings inside the resulting binary. If you compile with USE_TEST1 (-dUSE_TEST1) you will find the 'Test' string. If you compile with USE_TEST2 you'll find the 'TestSub' string and if you compile with USE_TEST3 you will find no string, because due to the call of Test3 with 1 instead of 2 the call to TestSub2 will be optimized as well.

This will also work across units and thus also the RTL.

jamie

  • Hero Member
  • *****
  • Posts: 4911
Re: To optimize RTL, make function PCharToInt
« Reply #5 on: July 23, 2021, 01:02:52 pm »
Then I will remove myself from the group because what I said still holds true.
Like it or not you seem to keep defending bugs and claimed features that just don't work.

 Also bad code generation with blanket statements in return.

Enjoy, it was nice while it lasted . I am sure that will make u happy.
The only true wisdom is knowing you know nothing

ccrause

  • Sr. Member
  • ****
  • Posts: 446
Re: To optimize RTL, make function PCharToInt
« Reply #6 on: July 23, 2021, 03:01:25 pm »
Please stop coming up with these badly thought out ideas.

It only aids in adding more useless redundant junk to an already over whelmed RTL..
I think the intent of @Alextp was to highlight the use of copying (sub)strings when it may not be necessary (e.g. when the copied string is not modified), not to suggest yet more functionality.  The following post clearly illustrates that there are existing alternatives that avoids making copies of strings:
These 3 can be opt'ed already:

In  fphttpclient.pp: LowerCase(Copy(S,1,Length(SetCookie)))=SetCookie
In fphttpclient.pp: ((LowerCase(Copy(HTTPHeaders[Result],1,l)))<>h)
In fpreport.pp: SameText(Copy(BaseName,1,9),'TFPReport')

Using StrLComp() and StrLIComp().

To me this suggestion seems sensible, but then I'm no expert on the low level details of string handling.

Perhaps @Alextp can come up with some benchmark tests to highlight potential improvements?

wp

  • Hero Member
  • *****
  • Posts: 8881
Re: To optimize RTL, make function PCharToInt
« Reply #7 on: July 23, 2021, 03:39:45 pm »
Such micro-optimizations usually are not noticable, but have the risk of breaking something somewhere else. I fully agree with jamie.
« Last Edit: July 23, 2021, 03:50:09 pm by wp »
Mainly Lazarus trunk / fpc 3.2.0 / all 32-bit on Win-10, but many more...

engkin

  • Hero Member
  • *****
  • Posts: 2952
Re: To optimize RTL, make function PCharToInt
« Reply #8 on: July 23, 2021, 04:28:46 pm »
1- I disagree with jamie.
2- "the risk of breaking something somewhere else" is not a good reason,  that same risk is there by simply touching the code anywhere, including fixing a bug.
3- Objecting to every "micro-optimization" will not enhance the speed. It is the collective result of all micro-optimizations that increases the speed.
4- Typically, every function that deals with strings should have a counterpart that takes pchar and length to allow for same usage even when a String type is used.


wp

  • Hero Member
  • *****
  • Posts: 8881
Re: To optimize RTL, make function PCharToInt
« Reply #9 on: July 23, 2021, 04:32:06 pm »
Typically, every function that deals with strings should have a counterpart that takes pchar and length to allow for same usage even when a String type is used.
The problem will arise when the string has a legal #0 inside. If not done properly the PChar version can stop parsing here and miss the rest of the string.
« Last Edit: July 23, 2021, 04:35:22 pm by wp »
Mainly Lazarus trunk / fpc 3.2.0 / all 32-bit on Win-10, but many more...

engkin

  • Hero Member
  • *****
  • Posts: 2952
Re: To optimize RTL, make function PCharToInt
« Reply #10 on: July 23, 2021, 04:41:03 pm »
I did mention "length" in the section you quoted, for this and to make the move from String type to pchar easy.

lucamar

  • Hero Member
  • *****
  • Posts: 4219
Re: To optimize RTL, make function PCharToInt
« Reply #11 on: July 23, 2021, 04:57:29 pm »
The problem will arise when the string has a legal #0 inside. If not done properly the PChar version can stop parsing here and miss the rest of the string.

Note that he said "[...] takes pchar and length [...]" so the problem doesn't need to arise. And if passing a "normal" string it's even easier since all one has to do is:
Code: Pascal  [Select][+][-]
  1. Somefunction(PChar(AString), Length(AString));
;)

ETA: As he just said. D***ed cross-posts! :-[
« Last Edit: July 23, 2021, 04:59:24 pm by lucamar »
Turbo Pascal 3 CP/M - Amstrad PCW 8256 (512 KB !!!) :P
Lazarus/FPC 2.0.8/3.0.4 & 2.0.12/3.2.0 - 32/64 bits on:
(K|L|X)Ubuntu 12..18, Windows XP, 7, 10 and various DOSes.

wp

  • Hero Member
  • *****
  • Posts: 8881
Re: To optimize RTL, make function PCharToInt
« Reply #12 on: July 23, 2021, 05:05:07 pm »
Yes, and if length were always respected we would not have all these buffer underruns in C. As I said: "if done properly"... "When things can go wrong they will go wrong." Or: "Never change a winning team".

I did a lot of these micro-optimizations myself with arguments like "it looks better when I do it this way", "ah, this is a bit faster" etc., and I almost always ended up in introducing a bug somewhere. This is different from fixing a bug. A bug MUST be fixed, micro-optimizations gains nothing.
Mainly Lazarus trunk / fpc 3.2.0 / all 32-bit on Win-10, but many more...

Alextp

  • Hero Member
  • *****
  • Posts: 1403
    • UVviewsoft
Re: To optimize RTL, make function PCharToInt
« Reply #13 on: July 23, 2021, 05:10:53 pm »
@wp,
Okay, if you think 'it gives nothing', let the DataTime code be as is.

engkin

  • Hero Member
  • *****
  • Posts: 2952
Re: To optimize RTL, make function PCharToInt
« Reply #14 on: July 23, 2021, 05:26:29 pm »
This is exactly what we are trying to do, to give you String functions in the RTL that are optimized so you don't need to do more micro-optimizations and introduce bugs into your code.

Forced or not, the reasoning behind rejecting "micro-optimizations" is not correct. Dealing with strings, pchar is the natural type to use, while String type is more of a user-friendly type to hide address, length, codepage and reference count.

Length should always be respected instead of being lazy and use Copy with MaxInt.

 

TinyPortal © 2005-2018