Jump to content
pyscripter

Smart Pointers - Generics vrs non-generic implementastion

Recommended Posts

On 1/3/2019 at 2:11 PM, Микола Петрівський said:

@Rudy Velthuis Nice idea, but does not work well with automatic type inference:


var Bob := SmartPtr<TTalking>(TTalking.Create('Bob'));

 

The compiler needs a type for Bob so it can know it should return a TTalking (through the Implicit operator) and not a TSmartPtr<TTalking>. There is no way around that.

 

But you can do without the inference:  

var Bob: TTalking := SmartPtr<TTalking>(TTalking.Create('Bob'));

 

Edited by Rudy Velthuis

Share this post


Link to post

@Stefan Glienke I am testing smart-pointers in Delphi Rio using Spring4D. Here is my testprogram. I created a generic TObjectlist and I want to add simple TObjects to this list using Shared.Make(TTestObj.Create). The problem is that whenever I add an object to the List the previous object is released. See the output of my program. Does anyone know how to solve this problem? Thanks in advance. 

program TestSmartPointer;

{$APPTYPE CONSOLE}

uses
  Spring,
  Diagnostics,
  Classes,
  SysUtils,
  System.Generics.Collections;

type
  TTestObj = class
  private
    FDescription: string;
  public
    property Description: string read FDescription write FDescription;
    destructor Destroy; override;
  end;
  TTestList = class(TObjectList<TTestObj>)
  public
    destructor Destroy; override;
  end;

procedure Test_SmartPointer;
begin
  Writeln('SmartPointer test started');
  var lTestList := Shared.Make(TTestList.Create)();
  lTestList.OwnsObjects := false;
  for var i := 1 to 10 do
  begin
    var lTestObj := Shared.Make(TTestObj.Create)();
//    var lTestObj := TTestObj.Create;
    lTestObj.Description := i.ToString;
    Writeln(format('TestObj with description %s added to Testlist', [lTestObj.Description]));
    lTestList.Add(lTestObj);
  end;
  Writeln('SmartPointer test finished');
end;

{ TTestObj }

destructor TTestObj.Destroy;
begin
  Writeln(format('TestObj with description %s is destroyed', [FDescription]));
  inherited;
end;

{ TTestList }

destructor TTestList.Destroy;
begin
  Writeln('TTestList is destroyed');
  inherited;
end;

begin
  Test_SmartPointer;
  Readln;
end.

 

2020-05-07 15_18_28-D__KDI_Projects_TestSmartPointer3_Win32_Debug_TestSmartPointer.exe.png

Share this post


Link to post
48 minutes ago, Kees Dijksman said:

@Stefan Glienke I am testing smart-pointers in Delphi Rio using Spring4D. Here is my testprogram. I created a generic TObjectlist and I want to add simple TObjects to this list using Shared.Make(TTestObj.Create). The problem is that whenever I add an object to the List the previous object is released. See the output of my program. Does anyone know how to solve this problem?

Cross posted: https://stackoverflow.com/questions/61656924/smartpointers-do-not-work-well-with-a-generic-tobjectlist-in-delphi

Share this post


Link to post
On 2/1/2019 at 1:07 AM, Rudy Velthuis said:

The compiler needs a type for Bob so it can know it should return a TTalking (through the Implicit operator) and not a TSmartPtr<TTalking>. There is no way around that.

If you want type inference then add a new method:

 

function SmartPtr<T>.Value: T;
begin
  if FGuard <> nil then
    Result := FGuard.Value
  else
    Result := nil;
end;

Then you can write:

    var Bob := SmartPtr<TTalking>(TTalking.Create('Bob')).Value;

slightly more elegant than having three times TTalking on the same line.

Share this post


Link to post
11 minutes ago, pyscripter said:

If you want type inference then add a new method

Um... you're responding to a post which is over a year old, written by a person who's no longer alive...

Share this post


Link to post
7 minutes ago, Anders Melander said:

Um... you're responding to a post which is over a year old, written by a person who's no longer alive...

His legacy lives on through people like me using his code...

  • Like 3

Share this post


Link to post

Minimalist implementation of SmartPointers based on a old post by Barry Kelly  comparable to the Spring4D one in performance.

 

type
  TObjectHandle<T: class> = class(TInterfacedObject, TFunc<T>)
  private
    FValue:  T;
  public
    constructor  Create(AValue:  T);
    destructor  Destroy;  override;
    function  Invoke:  T;
  end;

  TSmartPointer = record
    class function Make<T: class>(AValue: T): TFunc<T>; static;
  end;

constructor  TObjectHandle<T>.Create(AValue:  T);
begin
  FValue  :=  AValue;
end;

destructor  TObjectHandle<T>.Destroy;
begin
  FValue.Free;
end;

function  TObjectHandle<T>.Invoke:  T;
begin
  Result  :=  FValue;
end;


{ TSmartPointer }

class function TSmartPointer.Make<T>(AValue: T): TFunc<T>;
begin
  Result := TObjectHandle<T>.Create(AValue);
end;

Used as in:

 

 var Bob := TSmartPointer.Make(TTalking.Create('Bob'))();
or
 var Bob := TSmartPointer.Make(TTalking.Create('Bob'));

 

  • Like 1

Share this post


Link to post
10 hours ago, pyscripter said:

comparable to the Spring4D one in performance

hardly - its approx 25% to 50% slower because it has to create and destroy an entire object with all bells and whistles. 🙂

  • Like 1
  • Thanks 1

Share this post


Link to post
10 hours ago, pyscripter said:

Minimalist implementation of SmartPointers based on a old post by Barry Kelly  comparable to the Spring4D one in performance.

It is simpler, but not faster.

 

FWIW, I am using the same simple implementation because I don't need that extra speed. Smart pointer already brings in some performance drop and in places where I can live with that I can also live with unoptimized version of smart pointer. But if you really want to use smart pointers and you really need every last CPU cycle you can squeeze out of it, then Spring4D is the way to go. 

  • Like 1

Share this post


Link to post
4 minutes ago, Anders Melander said:

Kudos for that report Stefan. Very well written.

Too bad it will most likely rot in JIRA with the other 1900 open as "new feature" classified issues...

  • Sad 2

Share this post


Link to post
1 minute ago, Stefan Glienke said:

Too bad it will most likely rot in JIRA with the other 1900 open as "new feature" classified issues...

Bad morning? 🙂

On the bright side, and speaking of JIRA, Embarcadero are saints compared to Atlassian. The worst thing you can experience when trying to find out why something doesn't work in an Atlassian product is that it's already been reported and is in their JIRA. Because then you know that it's been there for ten years and will sit there for at least 5 more. Regardless of the thousands of votes and increasingly angry comments. I love the Atlassian stack but GodDamnThemToHell they're slow.

Share this post


Link to post
15 minutes ago, Anders Melander said:

Bad morning? 🙂

On the bright side, and speaking of JIRA, Embarcadero are saints compared to Atlassian. The worst thing you can experience when trying to find out why something doesn't work in an Atlassian product is that it's already been reported and is in their JIRA. Because then you know that it's been there for ten years and will sit there for at least 5 more. Regardless of the thousands of votes and increasingly angry comments. I love the Atlassian stack but GodDamnThemToHell they're slow.

No, actually pretty good, thank you. 🙂 It's just the experience with issues being reclassified and then "forgotten". And yes it can always be worse is also a way of optimism 😉

Share this post


Link to post
1 hour ago, David Heffernan said:

Like that works!

It could, if more people actually bothered to.
But, yeah... I hear you.

Share this post


Link to post
35 minutes ago, Lars Fosdal said:

It could, if more people actually bothered to.
But, yeah... I hear you.

It's a shockingly bad way for a software house to prioritise. It's susceptible to an effect known as self selection. As worthwhile as a Twitter poll. 

  • Like 1
  • Haha 1

Share this post


Link to post
7 hours ago, Stefan Glienke said:

hardly - its approx 25% to 50% slower

True.  Spring4D is the best!  And for what it is (not) worth, I have voted for RSP-27375.

 

Here is another one (more complex but still compact) which I think is comparable and similar in approach to Spring4D, based on a Stackoverflow question.  I am adding it here for the completeness of the discussion.

 

type
TInjectType<T> = record
public
  VMT: pointer;
  unknown: IInterface;
  RefCount: integer;
  AValue: T;
end;

TInject<T> = class
public type
  TInjectType = TInjectType<T>;
  PInjectType = ^TInjectType;
end;

PInjectObjectType = TInject<TObject>.PInjectType;

TSmartPointer = class
  class function Wrap<T: class>(const AValue: T): TFunc<T>; static;
end;

function Trick_Release(const obj:  PInjectObjectType): Integer; stdcall; forward;
function Trick_AddRef(const obj: PInjectObjectType): Integer; stdcall; forward;
function Invoke(const obj:  PInjectObjectType): TObject; forward;

const
  PSEUDO_VMT: array [0 .. 3] of pointer = (nil, @Trick_AddRef, @Trick_Release, @Invoke);

function Trick_AddRef(const obj: PInjectObjectType): Integer; stdcall;
begin
  Result:= AtomicIncrement(Obj^.RefCount);
end;

function Trick_Release(const obj:  PInjectObjectType): Integer; stdcall;
begin
  Result:= AtomicDecrement(Obj^.RefCount);
  WriteLn('Release '+IntToStr(Obj.RefCount));
  if Result = 0 then begin
    obj^.AValue.Free;
    FreeMem(obj);
  end;
end;

function Invoke(const obj:  PInjectObjectType): TObject;
begin
  Result:= obj^.AValue;
end;

class function TSmartPointer.Wrap<T>(const AValue: T): TFunc<T>;
var
  p: PInjectObjectType;
begin
  P:= GetMemory(SizeOf(TInjectType<T>));
  p.RefCount:= 1;
  pointer(p.unknown):= p;
  p.VMT:= @PSEUDO_VMT;
  p.AValue:= AValue;
  pointer(Result):= pointer(TFunc<T>(p));
end;

Note: typo corrected (see below).

Edited by pyscripter

Share this post


Link to post
1 hour ago, pyscripter said:

Here is another one (more complex but still compact) which I think is comparable and similar in approach to Spring4D, based on a Stackoverflow question.  I am adding it here for the completeness of the discussion.

Ugh  - just stop it please - first there is a typo resulting in an AV as TS in TSmartPointer.Wrap needs to be of type TInjectType<T> (fwiw these types are not necessary at all as you can just call New(p) and have the correct amount of memory allocated) and second this implementation will leak if you assign TSmartPointer.Wrap<T> to anything that is not nil already.

Edited by Stefan Glienke

Share this post


Link to post
56 minutes ago, Stefan Glienke said:

Ugh  - just stop it please

That is so nice...

 

Thanks for spotting the typo anyway.    With the typo fixed the following test code:

 

procedure Test();
begin
  try
    var Bob := TSmartPointer.Wrap(TTalking.Create('Bob'));
    Bob.Talk;
    var John := TSmartPointer.Wrap(TTalking.Create('John'));
    John.Talk;
    John := Bob;
    John.Talk;
  finally
    WriteLn('Do more stuff');
  end;
end;

produces

 

Bob is talking
John is talking
Release 0
John is gone
Bob is talking
Release 1
Release 0
Bob is gone
Do more stuff

with no AV or memory leak.

Edited by pyscripter

Share this post


Link to post
procedure Main;
begin
  var Bob := TSmartPointer.Wrap(TTalking.Create('Bob'));
  Bob.Talk;
  Bob := TSmartPointer.Wrap(TTalking.Create('Another bob'));
  Bob.Talk;
end;

Leak because Wrap just blasts the newly created smartpointer into result not caring if its already assigned - interface results are passed as var parameter and thus contain the value that was in them before the call. I am tired fixing code that other people post on SO that I already implemented properly myself. 🙂

Edited by Stefan Glienke
  • Like 1
  • Thanks 1

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×