Hello,

IMO there is an unnecessary Move() operation in fpc_AnsiStr_To_AnsiStr if (orgcp=cp).

fpc_AnsiStr_To_AnsiStr creates a copy of the AnsiString even if the destination and source codepages are equal. See:

program AnsiUtf8;
var
   Utf8Str: UTF8String;
   RawStr: RawByteString;
   Str: string;
begin
   DefaultSystemCodePage := CP_UTF8;
   Utf8Str := 'hello';
   Str := Utf8Str; // this makes a copy (fpc_AnsiStr_To_AnsiStr -> Move)
   RawStr := 'hello';
   SetCodePage(RawStr, CP_UTF8, False);
   Str := RawStr; // this doesn't make a copy
end.

Is there a reason for this? See the attached patch.

Thanks
Ondrej

It's probably what Delphi does as well. The result is that the refcount of a string after such an assignment is currently always one. I've had my share for now fighting with people who rely on implementation details (like this is one), so I'd rather not change that unless Delphi does it too (and even then we may get complaints that FPC is not backwards compatible in this respect).

See the attached patch.

Your patch will return an empty string if orgcp is different from both cp and CP_NONE.

Jonas

Personally I'm fine with people refactoring code in order to improve performance in either the compiler or the programs it compiles, just so long as it behaves exactly the same as before (which in this case, it doesn't because of the empty string issue that Jonas mentioned).

Although if I'm not mistaken, I always assume there's going to be a performance hit if strings are used outside of constants.

Gareth aka. Kit

---
This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus

Jonas Maebe wrote

On 28/04/2019 09:55, Ondrej Pokorny wrote: It's probably what Delphi does as well. The result is that the refcount of a string after such an assignment is currently always one.

Thanks for the answer. Yes, Delphi does the same. But strings have copy-on-write so the refcount value doesn't really matter - unless you change the resulting string via a PChar. Btw. changing a PAnsiChar/PChar results in a SIGSEGV in FPC but is OK in Delphi. Could you explain this?

program AnsiUtf8;
{$ifdef fpc}{$mode delphi}{$else}{$apptype console}{$endif}
var
   Utf8Str: UTF8String;
   Str: AnsiString;
   P: PAnsiChar;
begin
   DefaultSystemCodePage := 65001;
   Utf8Str := 'hello';
   Str := Utf8Str;
   P := PAnsiChar(Str);
   P[1] := 'x'; // SIGSEGV in FPC, OK in Delphi
   Writeln(Str);     // writes hxllo in Delphi
   Writeln(Utf8Str); // writes hello in Delphi
end.

The documentation doesn't tell anything about it:
https://www.freepascal.org/docs-html/ref/refsu12.html

If changing a string via a PChar is not allowed in FPC than the argument with refcount is not really valid.

Another thing: if you do the assignment directly (UTF8String -> AnsiString), you get a refcount of 1 (=a new copy of the string), but if you do the assignment via a RawByteString (UTF8String -> RawByteString -> AnsiString), you get a refcount of 3 (=the same string):

program AnsiUtf8;
{$ifdef fpc}{$mode delphi}{$else}{$apptype console}{$endif}
var
   Utf8Str: UTF8String;
   RawStr: RawByteString;
   Str: AnsiString;
begin
   DefaultSystemCodePage := 65001;
   Utf8Str := Copy('hello', 1);
   RawStr := Utf8Str;
   Str := RawStr;
   Writeln(PInteger(PByte(Str) - 8)^); // write refcount
end.

I've had my share for now fighting with people who rely on implementation details (like this is one), so I'd rather not change that unless Delphi does it too (and even then we may get complaints that FPC is not backwards compatible in this respect).

It's funny to see that the holy mantra of the "implementation detail" is used once to support a different behavior and the second time to fight it :)

> See the attached patch. Your patch will return an empty string if orgcp is different from both cp and CP_NONE.

This is nonsense. I didn't touch the last else-part that is used when orgcp is different from both cp and CP_NONE. You can easily check yourself:

program AnsiUtf8;
var
   Utf8Str: UTF8String;
   Str: AnsiString;
begin
   DefaultSystemCodePage := 1250;
   Utf8Str := 'hello';
   Str := Utf8Str;
   Writeln(Str);
end.

Ondrej

J. Gareth Moreton wrote

Personally I'm fine with people refactoring code in order to improve performance in either the compiler or the programs it compiles, just so long as it behaves exactly the same as before (which in this case, it doesn't because of the empty string issue that Jonas mentioned).

What empty string issue? Did I miss something?

program AnsiUtf8;
var
   Utf8Str: UTF8String;
   Str: AnsiString;
begin
   DefaultSystemCodePage := 1250;
   Utf8Str := 'hello';
   Str := Utf8Str;
   Writeln(Str);
end.

Ondrej

Probably not, as you've already addressed it yourself.  It was regarding Jonas' comment about the empty string being returned if orgcp is neither cp or CP_NONE - I didn't check it myself but went by his word.

Gareth

---
This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus

Exactly.

Btw. changing a PAnsiChar/PChar results in a SIGSEGV in FPC but is OK in Delphi. Could you explain this?

http://wiki.freepascal.org/User_Changes_3.0#Literal_storage_memory_has_been_made_read-only

If changing a string via a PChar is not allowed in FPC than the argument with refcount is not really valid.

It's only for string constants.

It's funny to see that the holy mantra of the "implementation detail" is used once to support a different behavior and the second time to fight it :)

When a new feature is added, we always try to be compatible with Delphi as much as possible, even at the implementation level. The reason is that an many Delphi users refuses to accept that there is a difference between a language definition and an implementation decision/detail.

Once a feature has been implemented in FPC, we also try to keep it the same as much as possible (except when needed to fix bugs), because many FPC users are of the same opinion as Delphi users in this regard (and even if they accept it, it's still no fun if your code breaks when you update the compiler, even if it is your own fault).

Sorry, I misread the patch.

Jonas

Str is not a string constant but it is a variable:

program PCharTest;
var
   Str: AnsiString;
   P: PAnsiChar;
begin
   Str := 'hello';
   P := PAnsiChar(Str);
   P[1] := 'x'; // SIGSEGV in FPC, OK in Delphi
end.

If Str points a read-only buffer, FPC has to recreate it and assign a read/write buffer at latest at this line:

   P := PAnsiChar(Str);

I reported it: https://bugs.freepascal.org/view.php?id=35461

Thanks for the feedback! Yes, I remember the big discussion about the case-else optimization for enumeration types.

But honestly that one (case-else) has been a common programming habit/mistake/whatever.

I don't think that anybody's code depends on the fact that String1 := String2;
generates once a copy of String2 and another time it only increases the refcount, depending on what 8-bit string types are used on both sides of the assignment.

Furthermore, you still have an implementation difference with Delphi about the const string assignment:

program ConstStringTest;
var
   Str: AnsiString;
begin
   Str := 'hello';
   Writeln(PInteger(PByte(Str) - 8)^); // 1 in Delphi, -1 in FPC
   ReadLn;
end.

This difference results in the bug #35461 I mentioned above.

Ondrej

FPC has never supported this. Before
https://bugs.freepascal.org/view.php?id=24088 was fixed (in 2013), this changed the string constant (so that further uses of the same string constant anywhere in the program would result in using this changed value). After that was fixed, it has always crashed.

You'd be surprised on what small details people depend to (micro-)optimise their code, in this case to e.g. by not adding (currently) unnecessary UniqueString calls.

Indeed. It's also documented (see the last remark on https://freepascal.org/docs-html/ref/refse24.html , although there seems to be something wrong with the formatting). Changing that would slow down the use of ansistring constants a lot though (which most people don't like either).

Jonas

OK, I understand now. We have the assignment:

S1 := S2;

There are (among others) these 2 cases:

1.) If S2 is a constant, S1 will point to a read-only buffer and the refcount will be -1. This is wanted because "changing that would slow down the use of ansistring constants a lot". It is not implementation-compatible with Delphi.

2.) If S2 is a UTF8String and S1 is an AnsiString with CP=UTF8, S1 gains a new copy of S2 with refcount = 1. This is wanted so because it is implementation-compatible with Delphi and people may depend on this. It slows down the use of the UTF8String type a lot.

This seems like a great ambivalence to me. But no problem, I'll change all method parameters that expect UTF8 strings from UTF8String to RawByteString and manually take care that only UTF8 data is fed to them.

Thanks
Ondrej

Ondrej Pokorny wrote

But no problem, I'll change all method parameters that expect UTF8 strings from UTF8String to RawByteString and manually take care that only UTF8 data is fed to them.

OK, I changed every single UTF8String to RawByteString and the performance gain was between 0% and 10%, depending on the test. So not really a big gain.

But when I run the same code in Delphi, it needs less than half the time of FPC (2 tests need 1/3 of FPC time, the third 1/2 of the time). I compared unmodified Release modes of Delphi and Lazarus.

The tests are all about parsing and creating UTF-8 text data. FPC doesn't need to do any conversions because I use FPC-native AnsiStrings with CP-UTF8, and Delphi needs to convert the input/output between UTF8/UTF16 (in Delphi I use the native UTF16 string type). Delphi is still much much faster.

I'll have to continue searching why the performance is so much better in Delphi...

Ondrej