View Issue Details

IDProjectCategoryView StatusLast Update
0038812pas2jsrtlpublic2021-04-30 15:54
Reporterhenrique Assigned ToMattias Gaertner  
PrioritynormalSeverityminorReproducibilityalways
Status assignedResolutionopen 
PlatformPas2JsOSWindows 
Summary0038812: Some fixes in RTTI unit
DescriptionI made some adjustments in RTTI unit.
1 - Now the RTTI context will use the global type of variable, so as not to load the type of each time it is requested.
2 - Refactored the loading of procedure information for a better implementation.
3 - Adjusted the loading of the parameters to load the information correctly, which was not doing correctly.
4 - Created the load control of procedure parameters, to be loaded only once.
TagsNo tags attached.
Fixed in Revision
Attached Files

Activities

henrique

2021-04-27 13:31

reporter  

0001-V-rios-ajustes-na-RTTI.patch (3,439 bytes)   
From 4a7de3a349e20eb97b74dbd8f8ae9bc3a205dea9 Mon Sep 17 00:00:00 2001
From: Henrique Gottardi Werlang <henriquewerlang@hotmail.com>
Date: Tue, 27 Apr 2021 08:23:40 -0300
Subject: [PATCH] =?UTF-8?q?V=C3=A1rios=20ajustes=20na=20RTTI.?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 packages/rtl/rtti.pas | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/packages/rtl/rtti.pas b/packages/rtl/rtti.pas
index 505b4949..098a91e2 100644
--- a/packages/rtl/rtti.pas
+++ b/packages/rtl/rtti.pas
@@ -165,6 +165,7 @@ type
   TRttiMethod = class(TRttiMember)
   private
     FParameters: TRttiParameterArray;
+    FParametersLoaded: Boolean;
 
     function GetIsAsyncCall: Boolean;
     function GetIsClassMethod: Boolean;
@@ -1187,13 +1188,13 @@ begin
   Name:=t.Name;
   if isModule(t.Module) then
     Name:=t.Module.Name+'.'+Name;
-  if FPool.hasOwnProperty(Name) then
-    Result:=TRttiType(FPool[Name])
+  if GRttiContext.FPool.hasOwnProperty(Name) then
+    Result:=TRttiType(GRttiContext.FPool[Name])
   else
     begin
     Result := RttiTypeClass[T.Kind].Create(aTypeInfo);
 
-    FPool[Name]:=Result;
+    GRttiContext.FPool[Name]:=Result;
     end;
 end;
 
@@ -1373,10 +1374,12 @@ end;
 
 function TRttiMethod.GetProcedureFlags: TProcedureFlags;
 const
-  PROCEDURE_FLAGS: array of NativeInt = (1, 2, 4, 8, 16);
+  PROCEDURE_FLAGS: array[TProcedureFlag] of NativeInt = (1, 2, 4, 8, 16);
 
 var
-  Flag, ProcedureFlags: NativeInt;
+  Flag: TProcedureFlag;
+
+  ProcedureFlags: NativeInt;
 
 begin
   ProcedureFlags := MethodTypeInfo.ProcSig.Flags;
@@ -1384,7 +1387,7 @@ begin
 
   for Flag := Low(PROCEDURE_FLAGS) to High(PROCEDURE_FLAGS) do
     if PROCEDURE_FLAGS[Flag] and ProcedureFlags > 0 then
-      Result := Result + [TProcedureFlag(Flag)];
+      Result := Result + [Flag];
 end;
 
 function TRttiMethod.GetReturnType: TRttiType;
@@ -1394,28 +1397,35 @@ end;
 
 procedure TRttiMethod.LoadParameters;
 const
-  FLAGS_CONVERSION: array[TParamFlag] of Integer = (1, 2, 4, 8, 16, 32);
+  FLAGS_CONVERSION: array[TParamFlag] of NativeInt = (1, 2, 4, 8, 16, 32);
 
 var
-  A, Flag: Integer;
+  A: Integer;
+
+  Flag: TParamFlag;
 
   Param: TProcedureParam;
 
   RttiParam: TRttiParameter;
 
+  MethodParams: TProcedureParams;
+
 begin
-  SetLength(FParameters, Length(MethodTypeInfo.ProcSig.Params));
+  FParametersLoaded := True;
+  MethodParams := MethodTypeInfo.ProcSig.Params;
+
+  SetLength(FParameters, Length(MethodParams));
 
   for A := Low(FParameters) to High(FParameters) do
   begin
-    Param := MethodTypeInfo.ProcSig.Params[A];
+    Param := MethodParams[A];
     RttiParam := TRttiParameter.Create;
     RttiParam.FName := Param.Name;
     RttiParam.FParamType := GRttiContext.GetType(Param.TypeInfo);
 
-    for Flag in FLAGS_CONVERSION do
-      if Flag and Param.Flags > 0 then
-        RttiParam.FFlags := RttiParam.FFlags + [TParamFlag(A)];
+    for Flag := Low(FLAGS_CONVERSION) to High(FLAGS_CONVERSION) do
+      if FLAGS_CONVERSION[Flag] and Param.Flags > 0 then
+        RttiParam.FFlags := RttiParam.FFlags + [Flag];
 
     FParameters[A] := RttiParam;
   end;
@@ -1423,7 +1433,7 @@ end;
 
 function TRttiMethod.GetParameters: TRttiParameterArray;
 begin
-  if not Assigned(FParameters) then
+  if not FParametersLoaded then
     LoadParameters;
 
   Result := FParameters;
-- 
2.31.1.windows.1

Sven Barth

2021-04-28 09:28

developer   ~0130633

Regarding the type pool: neither the way it was before nor how you fixed it, is how it should be.

The idea is that the pool is shared between all TRttiContext records and when the last TRttiContext instance goes out of scope all objects stored in the pool are released (this way one can in principle control the memory used by the Rtti system).

The original code was wrong, because it created a new pool with each Create as you rightfully remarked. But with your changes the types will never be released during the lifetime of the program.

Thus that should be fixed differently.

henrique

2021-04-29 12:34

reporter   ~0130654

I'm going to split the commit into 4 and I'm just going to leave the pool changes here.

Once I have a solution, I'll send you a new patch.

henrique

2021-04-29 16:33

reporter   ~0130659

I'm just sending the fix of the pool I made, which is not optimal, just to separate the changes.

I registered another entry with the change in the loading of the parameters.

I'll see a way to make this pool work as expected.
0001-Ajuste-pool-do-contexto.patch (917 bytes)   
From 13a32109e5a1609f685940e8bf78adc23ecf8cd9 Mon Sep 17 00:00:00 2001
From: Henrique Gottardi Werlang <henriquewerlang@hotmail.com>
Date: Thu, 29 Apr 2021 11:25:18 -0300
Subject: [PATCH] Ajuste pool do contexto.

---
 packages/rtl/rtti.pas | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/packages/rtl/rtti.pas b/packages/rtl/rtti.pas
index dad73d92..f993545e 100644
--- a/packages/rtl/rtti.pas
+++ b/packages/rtl/rtti.pas
@@ -1190,13 +1190,13 @@ begin
   Name:=t.Name;
   if isModule(t.Module) then
     Name:=t.Module.Name+'.'+Name;
-  if FPool.hasOwnProperty(Name) then
-    Result:=TRttiType(FPool[Name])
+  if GRttiContext.FPool.hasOwnProperty(Name) then
+    Result:=TRttiType(GRttiContext.FPool[Name])
   else
     begin
     Result := RttiTypeClass[T.Kind].Create(aTypeInfo);
 
-    FPool[Name]:=Result;
+    GRttiContext.FPool[Name]:=Result;
     end;
 end;
 
-- 
2.31.1.windows.1

Mattias Gaertner

2021-04-30 11:18

manager   ~0130672

I applied the 0001-Ajuste-pool-do-contexto.patch.
What about the other issues?

henrique

2021-04-30 12:56

reporter   ~0130676

I'm seeing how to do it, but I couldn't get back to it to check it out.

Sven Barth

2021-04-30 15:54

developer   ~0130678

@Mattias: henrique had split the big patch precisely so that the 0001-Ajuste-pool-do-contexto.patch does not need to be applied yet while the other changes can, because the resulting behaviour is still wrong.

Issue History

Date Modified Username Field Change
2021-04-27 13:31 henrique New Issue
2021-04-27 13:31 henrique File Added: 0001-V-rios-ajustes-na-RTTI.patch
2021-04-28 00:12 Mattias Gaertner Assigned To => Mattias Gaertner
2021-04-28 00:12 Mattias Gaertner Status new => assigned
2021-04-28 09:28 Sven Barth Note Added: 0130633
2021-04-29 12:34 henrique Note Added: 0130654
2021-04-29 16:33 henrique Note Added: 0130659
2021-04-29 16:33 henrique File Added: 0001-Ajuste-pool-do-contexto.patch
2021-04-30 11:18 Mattias Gaertner Note Added: 0130672
2021-04-30 12:56 henrique Note Added: 0130676
2021-04-30 15:54 Sven Barth Note Added: 0130678