Jump to content
pyscripter

Calling inherited in Destroy

Recommended Posts

In System.Classes we have the following:

 

destructor TList.Destroy;
begin
  Clear;
end;

I know that TObject.Destroy does nothing.  Is calling inherited Destroy from TObject direct descendants just a good practice or there are other considerations? 

Is the omission of the call to inherited just to save a few ms for the call to an empty procedure?

 

Share this post


Link to post

I can imagine the bug reports when some day TObject.Destroy actually gets some code and no one will remembers this oversight.

  • Like 2

Share this post


Link to post
10 hours ago, pyscripter said:

In System.Classes we have the following:

 


destructor TList.Destroy;
begin
  Clear;
end;

I know that TObject.Destroy does nothing.  Is calling inherited Destroy from TObject direct descendants just a good practice or there are other considerations? 

Is the omission of the call to inherited just to save a few ms for the call to an empty procedure?

 

 

Always call inherited in destructor. I would also add always call inherited in constructor, but if you miss to call constructor that does something, you will know soon enough. If you don't call destructor, you may have leaks which can be much harder to detect later on.

 

This is extremely low level optimization and it makes sense only in some code that will be called a lot, but even then think four times before you write it as changes in class declaration - using different ancestor can break such code badly. 

 

I have this kind of optimization in 3 places in my code in my smart pointer, weak and lazy reference implementations. Inherited is there, it is just commented out and there is additional comment explaining why it is there. If anything breaks in TObject, I have only one file to change.

 

But even now, while I am writing this I feel extremely uneasy about that code. So don't do write such code, unless you really have good reason, and don't write it even then.

  • Like 3
  • Thanks 1

Share this post


Link to post
11 hours ago, pyscripter said:

Is the omission of the call to inherited just to save a few ms for the call to an empty procedure?

I'd call it weird. It would only have somewhat > 0 sense if destroying VERY big number of objects. But this big number must be created first and dyn allocs will eat much more time than call to empty method, even to virtual one

Edited by Fr0sT.Brutal

Share this post


Link to post

Has anybody filed a bug report on this yet?

If nothing else, there should be a comment explaining why inherited Destroy isn't called.

  • Like 1

Share this post


Link to post

By the way TList.Destroy is called myriads of times by TWinControl.AlignContols.   I wonder whether using some other structure than a TList would make better sense.

Share this post


Link to post
2 hours ago, pyscripter said:

By the way TList.Destroy is called myriads of times by TWinControl.AlignContols.   I wonder whether using some other structure than a TList would make better sense.

Not really. It is the most lightweight list class that can be used there as dynamic arrays don't have capacity which prevents excessive memory reallocations when growing array.

 

Question is, whether the whole algorithm could be implemented in a different way, but that is another story (I am not saying that it can be better as I haven't done any analysis)

  • Like 1

Share this post


Link to post

Should the compiler not simply optimize away any inherited calls if there is noting to execute?

 

As a general discussion, calling or not calling inherited is a cool tool when you know what you are on about. But i think you all know that already.

Share this post


Link to post
7 minutes ago, Dany Marmur said:

Should the compiler not simply optimize away any inherited calls if there is noting to execute?

Yes it should. 

Share this post


Link to post

I inherit most of my classes from TObject (I have a habit of freeing what I create myself) and ALL of them have inherited calls in constructors and destructors, even if the Create was ReIntroduce-d. There are a MINIMAL amount of cases when this was strictly forbidden - there I did the same what @Dalija Prasnikar did - put it there, comment it out, put a comment why it is commented out. With links, if possible.

 

These just feel empty without inherited calls, even though I know they do nothing.

 

On the other hand, my OCD makes me capitalize almost every instruction (Create, If, Then, Begin, End, Procedure, Function, etc.) so it might be just plain stupid after all 🙂

 

  • Like 1

Share this post


Link to post
15 hours ago, Dany Marmur said:

Should the compiler not simply optimize away any inherited calls if there is noting to execute?

Should as in "it would be desirable" but the Delphi compiler does not do that.

This is usually achieved by compilers by either automatically inlining calls and then noticing there is nothing in such call hence no call/code to emit at all or by applying WPO or LTO. Both are things the Delphi compiler does not do.

The Delphi compiler does not inline inherited calls if the method is virtual which Destroy is.

Edited by Stefan Glienke
  • Sad 2

Share this post


Link to post

Given the simple code:

type
  TMyClass = class
  end;

procedure P;
begin
  var obj := TMyClass.Create;
  obj.Free;
end;

Try to count, how many function calls (and especially virtual calls) are performed here. And after that, it's become clear that additional "inherited" call in destructor changes almost nothing.

 

 

  • Like 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

×