Smart Pointers - Generics vrs non-generic implementastion

[Микола Петрівський 10]

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

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

 

[Rudy Velthuis 91]

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 January 31, 2019 by Rudy Velthuis

[Kees Dijksman 0]

@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.

 

[David Heffernan 2345]

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

[pyscripter 689]

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.

[Anders Melander 1782]

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...

[pyscripter 689]

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...

[pyscripter 689]

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'));

 

[Stefan Glienke 2002]

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. 🙂

[Dalija Prasnikar 1396]

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. 

[Stefan Glienke 2002]

4 minutes ago, Dalija Prasnikar said:

Smart pointer already brings in some performance drop

And some of it is unfortunately completely unnecessary and applies to every managed type! See https://quality.embarcadero.com/browse/RSP-27375

[Anders Melander 1782]

26 minutes ago, Stefan Glienke said:

See https://quality.embarcadero.com/browse/RSP-27375

Kudos for that report Stefan. Very well written.

[Stefan Glienke 2002]

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...

[Lars Fosdal 1791]

Vote like the feature depends on it!

[Anders Melander 1782]

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.

[Stefan Glienke 2002]

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 😉

[David Heffernan 2345]

1 hour ago, Lars Fosdal said:

Vote like the feature depends on it!

Like that works!

[Lars Fosdal 1791]

1 hour ago, David Heffernan said:

Like that works!

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

[David Heffernan 2345]

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. 

[pyscripter 689]

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 May 29, 2020 by pyscripter

[Stefan Glienke 2002]

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 May 29, 2020 by Stefan Glienke

[pyscripter 689]

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 May 29, 2020 by pyscripter

[Stefan Glienke 2002]

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 May 31, 2020 by Stefan Glienke