Recent

Author Topic: [solved] Strange behavior in generics.collections TDictionary  (Read 3466 times)

piola

  • Full Member
  • ***
  • Posts: 118
  • Lazarus 2.2, 64bit on Windows 8.1 x64
[solved] Strange behavior in generics.collections TDictionary
« on: January 26, 2022, 01:38:51 pm »
Consider the following code:

Code: Pascal  [Select][+][-]
  1. program project1;
  2.  
  3. {$mode objfpc}
  4. {$longstrings on} // see output below
  5. {$modeswitch advancedrecords}
  6.  
  7. uses Variants, Generics.Collections, SysUtils;
  8.  
  9. type TRecord = packed record
  10.   FID: NativeUInt;
  11.   FKey: String;
  12.   constructor Create (AID: NativeUInt; AKey: String);
  13. end;
  14.  
  15. constructor TRecord.Create (AID: NativeUInt; AKey: String);
  16. begin
  17.   FID := AID;
  18.   FKey := UpperCase (AKey);
  19. end;
  20.  
  21. var
  22.   Dict: specialize TDictionary<TRecord,Variant>;
  23.   i: specialize TPair<TRecord,Variant>;
  24.  
  25. begin
  26.   Dict := specialize TDictionary<TRecord,Variant>.Create;
  27.   Dict.Add (TRecord.Create (1, 'test'), 1);
  28.   for i in Dict do Writeln (i.Key.FID, #9, i.Key.FKey, #9, i.Value);
  29.   // ^^^ 1 TEST 1
  30.   // -> so the entry is ok!
  31.   Writeln (Dict.ContainsKey (TRecord.Create (1, 'test')));
  32.   // ^^^ with longstrings on -> FALSE
  33.   //     with longstrings off -> TRUE
  34.   Writeln (Dict.ContainsKey (TRecord.Create (1, 'TEST')));
  35.   // ^^^ always FALSE
  36.   Dict.Free;
  37. end.
  38.  

I'm very confused... I have no idea if I'm overseeing something or this is a bug in generics.collections or in the compiler.

My system is:
Lazarus 2.2.0 (64 bit), FPC 3.2.2 (64 bit) on Windows 8.1 (64 bit)
« Last Edit: January 26, 2022, 05:53:36 pm by piola »

Thaddy

  • Hero Member
  • *****
  • Posts: 14201
  • Probably until I exterminate Putin.
Re: Strange behavior in generics.collections TDictionary
« Reply #1 on: January 26, 2022, 02:16:23 pm »
generics.collections is compiled in{$H+} mode so longstrings are always on, unless you specialize explicitly to shortstring.
Note that your example outputs true/false here, both on Linux and Win64. You expected that?
Only thing I changed was to change string to shortstring and then tested with changing string to AnsiString.

BTW two remarks:
Syntax in capitals are a thing from before the Cretaceous. Please don't burden our eyes, we have reading glasses for that.
Variants can be used in generics, BUT then you are doing something wrong. Generics are to defeat variants, amongst other.
« Last Edit: January 26, 2022, 02:44:34 pm by Thaddy »
Specialize a type, not a var.

zamtmn

  • Hero Member
  • *****
  • Posts: 594
Re: Strange behavior in generics.collections TDictionary
« Reply #2 on: January 26, 2022, 02:39:07 pm »
Thaddy
Are you sure that's the problem? I'm not.
Code: Pascal  [Select][+][-]
  1.   Writeln (sizeof(TRecord));
  2.   Writeln (sizeof(i.Key));
returns the same values, regardless of $longstrings value

zamtmn

  • Hero Member
  • *****
  • Posts: 594
Re: Strange behavior in generics.collections TDictionary
« Reply #3 on: January 26, 2022, 02:47:16 pm »
TDictionary with key=record with longstring (or any data by pointer) can't work properly by design, use TMap or custom hash method

update:
it would be right to make a warning for such things
« Last Edit: January 26, 2022, 02:51:17 pm by zamtmn »

Thaddy

  • Hero Member
  • *****
  • Posts: 14201
  • Probably until I exterminate Putin.
Re: Strange behavior in generics.collections TDictionary
« Reply #4 on: January 26, 2022, 02:50:34 pm »
Thaddy
Are you sure that's the problem? I'm not.
Yes, I am sure.
Specialize a type, not a var.

avk

  • Hero Member
  • *****
  • Posts: 752
Re: Strange behavior in generics.collections TDictionary
« Reply #5 on: January 26, 2022, 03:37:32 pm »
I suppose the problem is that the default EqualityComparer seems to be comparing binary data in this case(record as key). See Generics.Defaults for details.
Accordingly, a specialized EqualityComparer should fix the situation.

piola

  • Full Member
  • ***
  • Posts: 118
  • Lazarus 2.2, 64bit on Windows 8.1 x64
Re: Strange behavior in generics.collections TDictionary
« Reply #6 on: January 26, 2022, 04:38:34 pm »
Syntax in capitals are a thing from before the Cretaceous. Please don't burden our eyes, we have reading glasses for that.

Sorry for that; I changed it now.

I suppose the problem is that the default EqualityComparer seems to be comparing binary data in this case(record as key). See Generics.Defaults for details.
Accordingly, a specialized EqualityComparer should fix the situation.

This is a good hint, thank you for that. I'll try and report back. (So if I understand it right, the comparison is based on pointers to the string which differ from call to call, right?)

piola

  • Full Member
  • ***
  • Posts: 118
  • Lazarus 2.2, 64bit on Windows 8.1 x64
Re: Strange behavior in generics.collections TDictionary
« Reply #7 on: January 26, 2022, 04:43:28 pm »
it would be right to make a warning for such things

When we are at warnings in generics.collections: I always get dozens of warning and notes when using generics.collections, for example:

generics.collections.pas(144,52) Note: Private type "TCustomPointersEnumerator$2<RRTYPES.TTerminal,RAILWAYRIVALSENGINE.TCustomList$1$crc5633D9C8.PT>.T" never used
generics.dictionaries.inc(191,92) Warning: Constructing a class "TCustomDictionaryEnumerator$4$crc19341810" with abstract method "DoMoveNext"
generics.dictionaries.inc(191,92) Warning: Constructing a class "TCustomDictionaryEnumerator$4$crc19341810" with abstract method "GetCurrent"
generics.dictionaries.inc(191,92) Warning: Constructing a class "TCustomDictionaryEnumerator$4$crc31956399" with abstract method "DoMoveNext"
generics.dictionaries.inc(191,92) Warning: Constructing a class "TCustomDictionaryEnumerator$4$crc31956399" with abstract method "GetCurrent"
generics.dictionaries.inc(191,92) Warning: Constructing a class "TCustomDictionaryEnumerator$4$crcC95B4680" with abstract method "DoMoveNext"
generics.dictionaries.inc(191,92) Warning: Constructing a class "TCustomDictionaryEnumerator$4$crcC95B4680" with abstract method "GetCurrent"
generics.dictionaries.inc(191,92) Warning: Constructing a class "TCustomDictionaryEnumerator$4$crc621585AD" with abstract method "DoMoveNext"
generics.dictionaries.inc(191,92) Warning: Constructing a class "TCustomDictionaryEnumerator$4$crc621585AD" with abstract method "GetCurrent"
generics.collections.pas(144,52) Note: Private type "TCustomPointersEnumerator$2<SYSTEM.AnsiString,RAILWAYRIVALSSERVER.TCustomList$1$crc6824777A.PT>.T" never used
generics.collections.pas(144,52) Note: Private type "TCustomPointersEnumerator$2<GRAPHTYPE.TGraphicsColor,RAILWAYRIVALSSERVER.TCustomList$1$crc9F59BD69.PT>.T" never used
generics.collections.pas(144,52) Note: Private type "TCustomPointersEnumerator$2<RAILWAYRIVALSSERVER.TPair$2<SYSTEM.AnsiString,GRAPHTYPE.TGraphicsColor>,RAILWAYRIVALSSERVER.TCustomList$1$crc566C6BBF.PT>.T" never used
generics.collections.pas(144,52) Note: Private type "TCustomPointersEnumerator$2<SYSTEM.NativeUInt,RAILWAYRIVALSSERVER.TCustomList$1$crc2A78E177.PT>.T" never used
generics.collections.pas(144,52) Note: Private type "TCustomPointersEnumerator$2<SYSTEM.LongInt,RAILWAYRIVALSSERVER.TCustomList$1$crc9F312717.PT>.T" never used
generics.collections.pas(144,52) Note: Private type "TCustomPointersEnumerator$2<RAILWAYRIVALSSERVER.TPair$2<SYSTEM.NativeUInt,SYSTEM.LongInt>,RAILWAYRIVALSSERVER.TCustomList$1$crcAB9DEB50.PT>.T" never used
generics.collections.pas(144,52) Note: Private type "TCustomPointersEnumerator$2<SYSTEM.NativeUInt,RAILWAYRIVALSCP.TCustomList$1$crc2A78E177.PT>.T" never used
generics.collections.pas(144,52) Note: Private type "TCustomPointersEnumerator$2<SYSTEM.Variant,RAILWAYRIVALSCP.TCustomList$1$crcE41905E8.PT>.T" never used


Is this normal?

zamtmn

  • Hero Member
  • *****
  • Posts: 594
Re: Strange behavior in generics.collections TDictionary
« Reply #8 on: January 26, 2022, 04:52:48 pm »
At the moment, the compiler does not generate a Warning, It is not implemented. I do not know if DELPHI does this. But it would be very nice to see the warning when trying to specialize unpacked or with data by pointer keys

piola

  • Full Member
  • ***
  • Posts: 118
  • Lazarus 2.2, 64bit on Windows 8.1 x64
Re: Strange behavior in generics.collections TDictionary
« Reply #9 on: January 26, 2022, 05:53:20 pm »
Okay, the advice to use IEqualityComparer did it. For others who may have the same problem, here is what I did:

Code: Pascal  [Select][+][-]
  1. program project1;
  2.  
  3. {$mode objfpc}
  4. {$longstrings on}
  5. {$modeswitch advancedrecords}
  6.  
  7. uses Variants, Generics.Collections, SysUtils, Generics.Defaults, Generics.Hashes;
  8.  
  9. type TRecord = packed record
  10.   FID: NativeUInt;
  11.   FKey: String;
  12.   constructor Create (AID: NativeUInt; AKey: String);
  13.   class operator = (a: TRecord; b: TRecord): Boolean;
  14. end;
  15.  
  16. constructor TRecord.Create (AID: NativeUInt; AKey: String);
  17. begin
  18.   FID := AID;
  19.   FKey := UpperCase (AKey);
  20. end;
  21.  
  22. class operator TRecord.= (a: TRecord; b: TRecord): Boolean;
  23. begin
  24.   Exit ((a.FID = b.FID) and (a.FKey = b.FKey));
  25. end;
  26.  
  27. type TMyComparer = class (TInterfacedObject, specialize IEqualityComparer<TRecord>)
  28.   function Equals (constref aLeft, aRight: TRecord): Boolean;
  29.   function GetHashCode (constref aValue: TRecord): UInt32;
  30. end;
  31.  
  32. function TMyComparer.Equals (constref aLeft, aRight: TRecord): Boolean;
  33. begin
  34.   Result := (aLeft = aRight);
  35. end;
  36.  
  37. function TMyComparer.GetHashCode(constref aValue: TRecord): UInt32;
  38. var
  39.   p: Pointer = NIL;
  40.   l: NativeUInt;
  41. begin
  42.   l := Length (AValue.FKey) + SizeOf (AValue.FID) + 1;
  43.   GetMem (p, l);
  44.   try
  45.     FillChar (p^, l, 0);
  46.     Move (AValue.FID, p^, SizeOf (AValue.FID));
  47.     StrPLCopy (p+SizeOf(AValue.FID), AValue.FKey, Length (AValue.FKey));
  48.     Exit (SimpleChecksumHash (p, l));
  49.   finally
  50.     FreeMem (p, l);
  51.   end;
  52. end;
  53.  
  54. var
  55.   Dict: specialize TDictionary<TRecord,Variant>;
  56.   i: specialize TPair<TRecord,Variant>;
  57.  
  58. begin
  59.   Dict := specialize TDictionary<TRecord,Variant>.Create (TMyComparer.Create);
  60.   Dict.Add (TRecord.Create (1, 'test'), 1);
  61.   for i in Dict do Writeln (i.Key.FID, #9, i.Key.FKey, #9, i.Value);
  62.   Writeln (Dict.ContainsKey (TRecord.Create (1, 'test')));
  63.   // ^^^ TRUE
  64.   Writeln (Dict.ContainsKey (TRecord.Create (1, 'TEST')));
  65.   // ^^^ TRUE
  66.   Writeln (Dict.ContainsKey (TRecord.Create (1, 'text')));
  67.   // ^^^ FALSE
  68.   Writeln (Dict.ContainsKey (TRecord.Create (2, 'TEST')));
  69.   // ^^^ FALSE
  70.   Dict.Free;
  71. end.
  72.  

It is not sufficient to declare the = operator for TRecord; one has to implement IEqualityComparer as well.
« Last Edit: January 26, 2022, 05:58:34 pm by piola »

avk

  • Hero Member
  • *****
  • Posts: 752
Re: [solved] Strange behavior in generics.collections TDictionary
« Reply #10 on: January 26, 2022, 06:10:41 pm »
I would like to note that the performance of a hash table is critically dependent on the quality of the hash function, and SimpleChecksumHash() is somehow not very good in this sense.

piola

  • Full Member
  • ***
  • Posts: 118
  • Lazarus 2.2, 64bit on Windows 8.1 x64
Re: [solved] Strange behavior in generics.collections TDictionary
« Reply #11 on: January 26, 2022, 06:22:38 pm »
I would like to note that the performance of a hash table is critically dependent on the quality of the hash function, and SimpleChecksumHash() is somehow not very good in this sense.

What would you recommend instead?

avk

  • Hero Member
  • *****
  • Posts: 752
Re: [solved] Strange behavior in generics.collections TDictionary
« Reply #12 on: January 26, 2022, 06:48:33 pm »
From what is available in Generics.Hashes, may be ххHash32?
Something like this:
Code: Pascal  [Select][+][-]
  1. function TMyComparer.GetHashCode(constref aValue: TRecord): UInt32;
  2. begin
  3.   if Length(aValue.FKey) <> 0 then
  4.     Result := xxHash32(
  5.                 xxHash32(42, @aValue.FID, SizeOf(aValue.FID)),
  6.                 @aValue.FKey[1], Length(aValue.FKey))
  7.   else
  8.     Result := xxHash32(
  9.                 xxHash32(42, @aValue.FID, SizeOf(aValue.FID)),
  10.                 nil, 0);
  11. end;
  12.  

PascalDragon

  • Hero Member
  • *****
  • Posts: 5446
  • Compiler Developer
Re: Strange behavior in generics.collections TDictionary
« Reply #13 on: January 27, 2022, 10:14:57 am »
it would be right to make a warning for such things

It is simply not possible to make a warning for some behaviour of user code, cause the generated code is correct. And for e.g. simple records without managed types it does work correctly.

 

TinyPortal © 2005-2018