Recent

« previous next »
Pages: [1]

Author Topic: [SOLVED] Improvement of TBinaryObjectWriter.WriteIdent  (Read 1937 times)

lagprogramming

  • Sr. Member
  • ****
  • Posts: 407
[SOLVED] Improvement of TBinaryObjectWriter.WriteIdent
« on: June 21, 2023, 10:24:36 am »
rtl/objpas/classes/writer.inc has procedure TBinaryObjectWriter.WriteIdent(const Ident: string);

Code: Pascal  [Select][+][-]
  1. procedure TBinaryObjectWriter.WriteIdent(const Ident: string);
  2. begin
  3.   { Check if Ident is a special identifier before trying to just write
  4.     Ident directly }
  5.   if UpperCase(Ident) = 'NIL' then
  6.     WriteValue(vaNil)
  7.   else if UpperCase(Ident) = 'FALSE' then
  8.     WriteValue(vaFalse)
  9.   else if UpperCase(Ident) = 'TRUE' then
  10.     WriteValue(vaTrue)
  11.   else if UpperCase(Ident) = 'NULL' then
  12.     WriteValue(vaNull) else
  13.   begin
  14.     WriteValue(vaIdent);
  15.     WriteStr(Ident);
  16.   end;
  17. end;
  18.  
The original code may compute multiple times UpperCase(Ident), even though Ident remains unchanged. The patch replaces the series of ifs with a "Case UpperCase(Ident) of".

Code: Pascal  [Select][+][-]
  1. diff --git a/rtl/objpas/classes/writer.inc b/rtl/objpas/classes/writer.inc
  2. index 93e2d23734..64536cb29f 100644
  3. --- a/rtl/objpas/classes/writer.inc
  4. +++ b/rtl/objpas/classes/writer.inc
  5. @@ -214,17 +214,12 @@ procedure TBinaryObjectWriter.WriteDate(const Value: TDateTime);
  6.  
  7.  procedure TBinaryObjectWriter.WriteIdent(const Ident: string);
  8.  begin
  9. -  { Check if Ident is a special identifier before trying to just write
  10. -    Ident directly }
  11. -  if UpperCase(Ident) = 'NIL' then
  12. -    WriteValue(vaNil)
  13. -  else if UpperCase(Ident) = 'FALSE' then
  14. -    WriteValue(vaFalse)
  15. -  else if UpperCase(Ident) = 'TRUE' then
  16. -    WriteValue(vaTrue)
  17. -  else if UpperCase(Ident) = 'NULL' then
  18. -    WriteValue(vaNull) else
  19. -  begin
  20. +  Case UpperCase(Ident) of
  21. +    'NIL'   : WriteValue(vaNil);
  22. +    'FALSE' : WriteValue(vaFalse);
  23. +    'TRUE'  : WriteValue(vaTrue);
  24. +    'NULL'  : WriteValue(vaNull);
  25. +    else
  26.      WriteValue(vaIdent);
  27.      WriteStr(Ident);
  28.    end;
« Last Edit: June 30, 2023, 01:27:36 pm by lagprogramming »
Logged

Josh

  • Hero Member
  • *****
  • Posts: 1375
Re: Improvement of TBinaryObjectWriter.WriteIdent
« Reply #1 on: June 21, 2023, 09:58:37 pm »
Hi

I thought Case with Strings was a ObjFpc only feature, not supported in delphi etc, unless I am wrong of course
Logged
The best way to get accurate information on the forum is to post something wrong and wait for corrections.

Bart

  • Hero Member
  • *****
  • Posts: 5575
    • Bart en Mariska's Webstek
Re: Improvement of TBinaryObjectWriter.WriteIdent
« Reply #2 on: June 21, 2023, 10:45:13 pm »
Quote from: Josh on June 21, 2023, 09:58:37 pm
I thought Case with Strings was a ObjFpc only feature, not supported in delphi etc, unless I am wrong of course
Yes, but why would that matter? It's source for fpc, not for Dephi.
Case of string gets translated by the compiler to multiple if..then..else IIRC.

A simpler apporach woud be to add a temporary variable to which UpperCase (Ident) and use that one for the comparisons.

Bart
Logged

lagprogramming

  • Sr. Member
  • ****
  • Posts: 407
Re: Improvement of TBinaryObjectWriter.WriteIdent
« Reply #3 on: June 22, 2023, 10:17:25 am »
Quote from: Bart on June 21, 2023, 10:45:13 pm
Quote from: Josh on June 21, 2023, 09:58:37 pm
I thought Case with Strings was a ObjFpc only feature, not supported in delphi etc, unless I am wrong of course
Yes, but why would that matter? It's source for fpc, not for Dephi.
Case of string gets translated by the compiler to multiple if..then..else IIRC.

A simpler apporach woud be to add a temporary variable to which UpperCase (Ident) and use that one for the comparisons.

Bart

Using the case statement I see a single call to uppercase. Assuming I add a new variable to store the uppercase value, do you think the pascal code would have an increased readability with a series of if then elses instead of a single case?
Code: Pascal  [Select][+][-]
  1. CLASSES$_$TBINARYOBJECTWRITER_$__$$_WRITEIDENT$ANSISTRING
  2. 00000000004B7A20 53                       push rbx
  3. 00000000004B7A21 4154                     push r12
  4. 00000000004B7A23 488D642488               lea rsp,[rsp-$78]
  5. 00000000004B7A28 4889FB                   mov rbx,rdi
  6. 00000000004B7A2B 4989F4                   mov r12,rsi
  7. 00000000004B7A2E 48C744246800000000       mov qword ptr [rsp+$68],$00000000
  8. 00000000004B7A37 48C744246000000000       mov qword ptr [rsp+$60],$00000000
  9. 00000000004B7A40 4889E2                   mov rdx,rsp
  10. 00000000004B7A43 488D742418               lea rsi,[rsp+$18]
  11. 00000000004B7A48 BF01000000               mov edi,$00000001
  12. 00000000004B7A4D E84E87F7FF               call -$000878B2    # $00000000004301A0 fpc_pushexceptaddr
  13. 00000000004B7A52 4889C7                   mov rdi,rax
  14. 00000000004B7A55 E88665F6FF               call -$00099A7A    # $000000000041DFE0 fpc_setjmp
  15. 00000000004B7A5A 89442458                 mov [rsp+$58],eax
  16. 00000000004B7A5E 85C0                     test eax,eax
  17. 00000000004B7A60 0F85E2000000             jnz +$000000E2    # $00000000004B7B48 CLASSES$_$TBINARYOBJECTWRITER_$__$$_WRITEIDENT$ANSISTRING+296
  18. 00000000004B7A66 488D7C2460               lea rdi,[rsp+$60]
  19. 00000000004B7A6B E8A0F3F6FF               call -$00090C60    # $0000000000426E10 FPC_ANSISTR_DECR_REF
  20. 00000000004B7A70 488D7C2468               lea rdi,[rsp+$68]
  21. 00000000004B7A75 4C89E6                   mov rsi,r12
  22. 00000000004B7A78 E833260100               call +$00012633    # $00000000004CA0B0 SYSUTILS_$$_UPPERCASE$ANSISTRING$$ANSISTRING
  23. 00000000004B7A7D 488B742468               mov rsi,[rsp+$68]
  24. 00000000004B7A82 488D7C2460               lea rdi,[rsp+$60]
  25. 00000000004B7A87 E8E4F3F6FF               call -$00090C1C    # $0000000000426E70 fpc_ansistr_assign
  26. 00000000004B7A8C 488D357D852F00           lea rsi,[rip+$002F857D]
  27. 00000000004B7A93 488B7C2460               mov rdi,[rsp+$60]
  28. 00000000004B7A98 E8B303F7FF               call -$0008FC4D    # $0000000000427E50 FPC_ANSISTR_COMPARE_EQUAL
  29. 00000000004B7A9D 4885C0                   test rax,rax
  30. 00000000004B7AA0 7516                     jnz +$16    # $00000000004B7AB8 CLASSES$_$TBINARYOBJECTWRITER_$__$$_WRITEIDENT$ANSISTRING+152
  31. 00000000004B7AA2 BE0D000000               mov esi,$0000000D
  32. 00000000004B7AA7 4889DF                   mov rdi,rbx
  33. 00000000004B7AAA E851080000               call +$00000851    # $00000000004B8300 CLASSES$_$TBINARYOBJECTWRITER_$__$$_WRITEVALUE$TVALUETYPE
  34. 00000000004B7AAF E994000000               jmp +$00000094    # $00000000004B7B48 CLASSES$_$TBINARYOBJECTWRITER_$__$$_WRITEIDENT$ANSISTRING+296
  35. 00000000004B7AB4 0F1F4000                 nop dword ptr [rax+$00]
  36. 00000000004B7AB8 488D3521852F00           lea rsi,[rip+$002F8521]
  37. 00000000004B7ABF 488B7C2460               mov rdi,[rsp+$60]
  38. 00000000004B7AC4 E88703F7FF               call -$0008FC79    # $0000000000427E50 FPC_ANSISTR_COMPARE_EQUAL
  39. 00000000004B7AC9 4885C0                   test rax,rax
  40. 00000000004B7ACC 7512                     jnz +$12    # $00000000004B7AE0 CLASSES$_$TBINARYOBJECTWRITER_$__$$_WRITEIDENT$ANSISTRING+192
  41. 00000000004B7ACE BE08000000               mov esi,$00000008
  42. 00000000004B7AD3 4889DF                   mov rdi,rbx
  43. 00000000004B7AD6 E825080000               call +$00000825    # $00000000004B8300 CLASSES$_$TBINARYOBJECTWRITER_$__$$_WRITEVALUE$TVALUETYPE
  44. 00000000004B7ADB E968000000               jmp +$00000068    # $00000000004B7B48 CLASSES$_$TBINARYOBJECTWRITER_$__$$_WRITEIDENT$ANSISTRING+296
  45. 00000000004B7AE0 488D3511852F00           lea rsi,[rip+$002F8511]
  46. 00000000004B7AE7 488B7C2460               mov rdi,[rsp+$60]
  47. 00000000004B7AEC E85F03F7FF               call -$0008FCA1    # $0000000000427E50 FPC_ANSISTR_COMPARE_EQUAL
  48. 00000000004B7AF1 4885C0                   test rax,rax
  49. 00000000004B7AF4 7512                     jnz +$12    # $00000000004B7B08 CLASSES$_$TBINARYOBJECTWRITER_$__$$_WRITEIDENT$ANSISTRING+232
  50. 00000000004B7AF6 BE09000000               mov esi,$00000009
  51. 00000000004B7AFB 4889DF                   mov rdi,rbx
  52. 00000000004B7AFE E8FD070000               call +$000007FD    # $00000000004B8300 CLASSES$_$TBINARYOBJECTWRITER_$__$$_WRITEVALUE$TVALUETYPE
  53. 00000000004B7B03 EB43                     jmp +$43    # $00000000004B7B48 CLASSES$_$TBINARYOBJECTWRITER_$__$$_WRITEIDENT$ANSISTRING+296
  54. 00000000004B7B05 0F1F00                   nop dword ptr [rax]
  55. 00000000004B7B08 488D35B9842F00           lea rsi,[rip+$002F84B9]
  56. 00000000004B7B0F 488B7C2460               mov rdi,[rsp+$60]
  57. 00000000004B7B14 E83703F7FF               call -$0008FCC9    # $0000000000427E50 FPC_ANSISTR_COMPARE_EQUAL
  58. 00000000004B7B19 4885C0                   test rax,rax
  59. 00000000004B7B1C 7512                     jnz +$12    # $00000000004B7B30 CLASSES$_$TBINARYOBJECTWRITER_$__$$_WRITEIDENT$ANSISTRING+272
  60. 00000000004B7B1E 31F6                     xor esi,esi
  61. 00000000004B7B20 4889DF                   mov rdi,rbx
  62. 00000000004B7B23 E8D8070000               call +$000007D8    # $00000000004B8300 CLASSES$_$TBINARYOBJECTWRITER_$__$$_WRITEVALUE$TVALUETYPE
  63. 00000000004B7B28 EB1E                     jmp +$1E    # $00000000004B7B48 CLASSES$_$TBINARYOBJECTWRITER_$__$$_WRITEIDENT$ANSISTRING+296
  64. 00000000004B7B2A 660F1F440000             nop word ptr [rax+rax+$00]
  65. 00000000004B7B30 BE07000000               mov esi,$00000007
  66. 00000000004B7B35 4889DF                   mov rdi,rbx
  67. 00000000004B7B38 E8C3070000               call +$000007C3    # $00000000004B8300 CLASSES$_$TBINARYOBJECTWRITER_$__$$_WRITEVALUE$TVALUETYPE
  68. 00000000004B7B3D 4C89E6                   mov rsi,r12
  69. 00000000004B7B40 4889DF                   mov rdi,rbx
  70. 00000000004B7B43 E8D8070000               call +$000007D8    # $00000000004B8320 CLASSES$_$TBINARYOBJECTWRITER_$__$$_WRITESTR$ANSISTRING
  71. 00000000004B7B48 E89389F7FF               call -$0008766D    # $00000000004304E0 fpc_popaddrstack
  72. 00000000004B7B4D 488D7C2468               lea rdi,[rsp+$68]
  73. 00000000004B7B52 E8B9F2F6FF               call -$00090D47    # $0000000000426E10 FPC_ANSISTR_DECR_REF
  74. 00000000004B7B57 488D7C2460               lea rdi,[rsp+$60]
  75. 00000000004B7B5C E8AFF2F6FF               call -$00090D51    # $0000000000426E10 FPC_ANSISTR_DECR_REF
  76. 00000000004B7B61 837C245800               cmp dword ptr [rsp+$58],$00
  77. 00000000004B7B66 740F                     jz +$0F    # $00000000004B7B77 CLASSES$_$TBINARYOBJECTWRITER_$__$$_WRITEIDENT$ANSISTRING+343
  78. 00000000004B7B68 E8238BF7FF               call -$000874DD    # $0000000000430690 FPC_RERAISE
  79. 00000000004B7B6D C744245800000000         mov [rsp+$58],$00000000
  80. 00000000004B7B75 EBD1                     jmp -$2F    # $00000000004B7B48 CLASSES$_$TBINARYOBJECTWRITER_$__$$_WRITEIDENT$ANSISTRING+296
  81. 00000000004B7B77 488D642478               lea rsp,[rsp+$78]
  82. 00000000004B7B7C 415C                     pop r12
  83. 00000000004B7B7E 5B                       pop rbx
  84. 00000000004B7B7F C3                       ret
Logged

Josh

  • Hero Member
  • *****
  • Posts: 1375
Re: Improvement of TBinaryObjectWriter.WriteIdent
« Reply #4 on: June 22, 2023, 10:50:11 am »
Personally i prefer the existing (mainly because I dont use Case Statements with Strings, i use if then else so its naturally readable to me, others will disagree  ;)). but adding a temp var for optimize speed would be fine.

Code: Pascal  [Select][+][-]
  1. procedure TBinaryObjectWriter.WriteIdent(const Ident: string);
  2. var TempIdent:String;
  3. begin
  4.   { Check if Ident is a special identifier before trying to just write
  5.     Ident directly }
  6.   TempIdent:=UpperCase(Ident);
  7.   if TempIdent = 'NIL' then
  8.     WriteValue(vaNil)
  9.   else if TempIdent = 'FALSE' then
  10.     WriteValue(vaFalse)
  11.   else if TempIdent = 'TRUE' then
  12.     WriteValue(vaTrue)
  13.   else if TempIdent = 'NULL' then
  14.     WriteValue(vaNull)
  15.   else
  16.   begin
  17.     WriteValue(vaIdent);
  18.     WriteStr(Ident);
  19.   end;
  20. end;      

for readability curious as to why your removing the comment.
Code: Pascal  [Select][+][-]
  1. -  { Check if Ident is a special identifier before trying to just write
  2. -    Ident directly }
« Last Edit: June 22, 2023, 11:06:35 am by Josh »
Logged
The best way to get accurate information on the forum is to post something wrong and wait for corrections.

lagprogramming

  • Sr. Member
  • ****
  • Posts: 407
Re: Improvement of TBinaryObjectWriter.WriteIdent
« Reply #5 on: June 22, 2023, 12:28:37 pm »
Quote from: Josh on June 22, 2023, 10:50:11 am
for readability curious as to why your removing the comment.
Code: Pascal  [Select][+][-]
  1. -  { Check if Ident is a special identifier before trying to just write
  2. -    Ident directly }

When I removed it I was thinking that the comment makes sense only in the presence of the if then else series, not in the presence of a case statement. The presence of the case statement would imply the contents of the existing comment because that's how the case statements work.
Logged

Josh

  • Hero Member
  • *****
  • Posts: 1375
Re: Improvement of TBinaryObjectWriter.WriteIdent
« Reply #6 on: June 22, 2023, 12:45:34 pm »
I find the purpose of the comment is to explain what the following code does; without having to analyze the code to work out what its doing.
Logged
The best way to get accurate information on the forum is to post something wrong and wait for corrections.

Thaddy

  • Hero Member
  • *****
  • Posts: 17361
  • Ceterum censeo Trump esse delendam
Re: Improvement of TBinaryObjectWriter.WriteIdent
« Reply #7 on: June 22, 2023, 12:56:46 pm »
The case statement with strings is actually translated by the compiler into if then else.
Also note the introductary notes about this "feature". Any way you see it, case with string introduces an indirection.
There was quite a discussion on the development forum about this too.
Logged
Due to censorship, I changed this to "Nelly the Elephant". Keeps the message clear.

AlexTP

  • Hero Member
  • *****
  • Posts: 2577
    • UVviewsoft
Re: Improvement of TBinaryObjectWriter.WriteIdent
« Reply #8 on: June 29, 2023, 09:33:19 pm »
Logged

Stefan Glienke

  • New Member
  • *
  • Posts: 24
Re: [SOLVED] Improvement of TBinaryObjectWriter.WriteIdent
« Reply #9 on: July 04, 2023, 11:33:26 am »
Anyone who fancies compiler optimization might want to look into switch lowering - see https://www.youtube.com/watch?v=gMqSinyL8uk
Logged
Pages: [1]
« previous next »
 

TinyPortal © 2005-2018