[PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

silvioprog
Hello,

I have a friend here in Brazil that added the `notify` support to the FGL lists, however he sented it to this Github PR [1].

I'm contacting him to explain how to send it to FPC bugtracker, so you can disconsider this PR at Github. :-)

Thank you!


--
Silvio Clécio

_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

Sven Barth-2

Am 19.02.2016 12:45 schrieb "silvioprog" <[hidden email]>:
>
> Hello,
>
> I have a friend here in Brazil that added the `notify` support to the FGL lists, however he sented it to this Github PR [1].

I'm against that. That would negatively impact the performance of these lists.

Regards,
Sven


_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

silvioprog
On Fri, Feb 19, 2016 at 10:24 AM, Sven Barth <[hidden email]> wrote:

Am 19.02.2016 12:45 schrieb "silvioprog" <[hidden email]>:
>
> Hello,
>
> I have a friend here in Brazil that added the `notify` support to the FGL lists, however he sented it to this Github PR [1].

I'm against that. That would negatively impact the performance of these lists.

As I said, please discard this PR, because I'm contacting the member (Fabiano) to use mantis instead of Github, and I received other patch from other guy (Gilson) improving this changes.

--
Silvio Clécio

_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

Sven Barth-2

Am 19.02.2016 14:45 schrieb "silvioprog" <[hidden email]>:
>
> On Fri, Feb 19, 2016 at 10:24 AM, Sven Barth <[hidden email]> wrote:
>>
>> Am 19.02.2016 12:45 schrieb "silvioprog" <[hidden email]>:
>> >
>> > Hello,
>> >
>> > I have a friend here in Brazil that added the `notify` support to the FGL lists, however he sented it to this Github PR [1].
>>
>> I'm against that. That would negatively impact the performance of these lists.
>
> As I said, please discard this PR, because I'm contacting the member (Fabiano) to use mantis instead of Github, and I received other patch from other guy (Gilson) improving this changes.

It doesn't matter whether it is requested over here or on Mantis. I'm against adding notifications to the generic lists, because that will degrade their performance.

Regards,
Sven


_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

silvioprog
On Fri, Feb 19, 2016 at 4:20 PM, Sven Barth <[hidden email]> wrote:
[...]

It doesn't matter whether it is requested over here or on Mantis. I'm against adding notifications to the generic lists, because that will degrade their performance.

Can you send little bit sample showing this degradation? I can inform him to not send the patch because this problem. (I think that him isn't registered here in the list yet)

--
Silvio Clécio

_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

Sven Barth-2

Am 19.02.2016 20:47 schrieb "silvioprog" <[hidden email]>:
>
> On Fri, Feb 19, 2016 at 4:20 PM, Sven Barth <[hidden email]> wrote:
> [...]
>
>> It doesn't matter whether it is requested over here or on Mantis. I'm against adding notifications to the generic lists, because that will degrade their performance.
>
> Can you send little bit sample showing this degradation? I can inform him to not send the patch because this problem. (I think that him isn't registered here in the list yet)

There's no need for a sample. This degradation is the whole reason why the non-generic classes TFPObjectList and TFPList exist compared to TObjectList and TList which do have notifications.

Regards,
Sven


_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

Graeme Geldenhuys-6
On 2016-02-19 23:04, Sven Barth wrote:
> There's no need for a sample. This degradation is the whole reason why the
> non-generic classes TFPObjectList and TFPList exist compared to TObjectList
> and TList which do have notifications.

I don't use Generics so it doesn't bother me either way. But purely from
a software design point of view, it should be quite easy to create a
list where notification can be enabled or disabled at runtime - maybe as
a parameter to the constructor or via a property. On disabling
notification there should be near zero degradation - at least not
something noticeable.

Just my 2¢ worth.


Regards,
  - Graeme -

_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

Michalis Kamburelis-3
In reply to this post by Sven Barth-2
>> Can you send little bit sample showing this degradation? I can inform him
>> to not send the patch because this problem. (I think that him isn't
>> registered here in the list yet)
>
> There's no need for a sample. This degradation is the whole reason why the
> non-generic classes TFPObjectList and TFPList exist compared to TObjectList
> and TList which do have notifications.

Hm, two points.

1. I'm using FGL containers a lot (because I like type-safety given by
generics), and I agree that adding a notification mechanism there can
slow them down. Mostly at allocation (because a deallocation is
already burdened down by having to do Deref() on every item, so it
would probably not suffer *so much* with unassigned OnNotify).

2. But this should not stop us from making a "generic container with
notification" classes. Maybe as new classes, maybe as a boolean toggle
to existing classes (as Graeme nicely suggests).

  Right now we only have "with notifications, and not type-safe"
containers (TObjectList/TList) and "without notifications, and
type-safe using generics" containers (TFPObjectList/TFPList). But
these two features (1. notifications and 2. type-safety thanks to
generics) should be independent. It makes sense to create a "with
notifications, and type-safe using generics" option.

  So I would not reject the idea so quickly. But it indeed should be
implemented carefully, without slowing down the existing
TFPObjectList/TFPList.

Regards,
Michalis
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

Maciej Izak
In reply to this post by silvioprog

Notifications events already works in Generics.Collections. ;P


_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

Sven Barth-2

Am 20.02.2016 08:55 schrieb "Maciej Izak" <[hidden email]>:
>
> Notifications events already works in Generics.Collections. ;P
>

Which reminds me that I wanted to work on integrating them...

Regards,
Sven


_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

[PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

Serguei TARASSOV
In reply to this post by Sven Barth-2
On 19/02/2016 20:20, [hidden email] wrote:

> Date: Fri, 19 Feb 2016 14:24:30 +0100
> From: Sven Barth<[hidden email]>
> To: FPC-Pascal users discussions<[hidden email]>
> Subject: Re: [fpc-pascal] [PATCH] Addition of OnNotify event on
> "TFPGObjectList" and "TFPGList" triggered on the insert, delete and
> extract methods.
>
> Am 19.02.2016 12:45 schrieb "silvioprog"<[hidden email]>:
>> >
>> >Hello,
>> >
>> >I have a friend here in Brazil that added the `notify` support to the FGL
> lists, however he sented it to this Github PR [1].
>
> I'm against that. That would negatively impact the performance of these
> lists.
>
> Regards,
> Sven
I agree.
Actually TFPGList has already an overhead vs TList (related tests
http://arbinada.com/main/en/node/1411) and it is not a good idea to
increase it.
Why not simple create subclass to implement events?

Regards,
Serguei
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
--
Regards,
Serguei
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

Michael Van Canneyt
In reply to this post by Michalis Kamburelis-3


On Sat, 20 Feb 2016, Michalis Kamburelis wrote:

>>> Can you send little bit sample showing this degradation? I can inform him
>>> to not send the patch because this problem. (I think that him isn't
>>> registered here in the list yet)
>>
>> There's no need for a sample. This degradation is the whole reason why the
>> non-generic classes TFPObjectList and TFPList exist compared to TObjectList
>> and TList which do have notifications.
>
> Hm, two points.
>
> 1. I'm using FGL containers a lot (because I like type-safety given by
> generics), and I agree that adding a notification mechanism there can
> slow them down. Mostly at allocation (because a deallocation is
> already burdened down by having to do Deref() on every item, so it
> would probably not suffer *so much* with unassigned OnNotify).
>
> 2. But this should not stop us from making a "generic container with
> notification" classes. Maybe as new classes, maybe as a boolean toggle
> to existing classes (as Graeme nicely suggests).
>
>  Right now we only have "with notifications, and not type-safe"
> containers (TObjectList/TList) and "without notifications, and
> type-safe using generics" containers (TFPObjectList/TFPList).

These latter 2 are not generics based. There was some experimental code
which attempted to implement TFPObjectList/TFPList on top of generics,
but they were slower than the native implementation. That experiment is
unmaintained.

TFPGList is the one you meant, probably.

Michael.
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

denisgolovan
In reply to this post by Sven Barth-2
Hi
 
That always keeps me wondering - why neither FPC, no any other mainstream static-type language implements specialization based not only on types, but on compile-time constants as well?
 
I mean something like
 
  TGenericList<T,options> = class
    ...
  end;
 
procedure TGenericList<T,options>.Delete(index:integer);
begin
  if (nnNotifyDelete in options) and Assigned(FOnDelete) then
     FOnDelete(...);
  end;
end;
 
// usage
type
  TMyList = specialize TGenericList<integer, [nnNotifyAdd, nnNotifyDelete, nnNotifyChange]>;
 
... that "if (nnNotifyDelete in options) " condition will optimized out by compiler if options lack corresponding enum constant.
Of course, FOnDelete property should be omitted as well in ideal case. That might require something like "static if" in D.
 
I've done that trick for ages using m4 preprocessor and it works really good, why it's still not supported natively by compilers themselves?
 
20.02.2016, 02:05, "Sven Barth" <[hidden email]>:
There's no need for a sample. This degradation is the whole reason why the non-generic classes TFPObjectList and TFPList exist compared to TObjectList and TList which do have notifications.

Regards,
Sven

,

_______________________________________________
fpc-pascal maillist - [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal

 
 
--
Regards,
Denis Golovan
 

_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

denisgolovan
In reply to this post by Sven Barth-2
20.02.2016, 02:05, "Sven Barth" <[hidden email]>:
> There's no need for a sample. This degradation is the whole reason why the non-generic classes TFPObjectList and TFPList exist compared to TObjectList and TList which do have notifications.
>
> Regards,
> Sven

/ My previous mail appeared to be in html format. Sorry for that.

Hi

That always keeps me wondering - why neither FPC, no any other mainstream static-type language implements specialization based not only on types, but on compile-time constants as well?

I mean something like

  TGenericList<T,options> = class
    ...
  end;

procedure TGenericList<T,options>.Delete(index:integer);
begin
  if (nnNotifyDelete in options) and Assigned(FOnDelete) then
    FOnDelete(...);
  end;
end;

// usage
type
  TMyList = specialize TGenericList<integer, [nnNotifyAdd, nnNotifyDelete, nnNotifyChange]>;

... that "if (nnNotifyDelete in options) " condition will optimized out by compiler if options lack corresponding enum constant.
Of course, FOnDelete property should be omitted as well in ideal case. That might require something like "static if" in D.

I've done that trick for ages using m4 preprocessor and it works really good, why it's still not supported natively by compilers themselves?

--
Regards,
Denis Golovan
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

Graeme Geldenhuys-6
In reply to this post by Michalis Kamburelis-3
On 2016-02-20 05:05, Michalis Kamburelis wrote:
>   So I would not reject the idea so quickly. But it indeed should be
> implemented carefully

And it will take 5-10 minutes to write some unit tests to see exactly
what speed penalty would occur. Why guess or assume. Write some unit
tests and get the facts! Then go from there.

Regards,
  - Graeme -

--
fpGUI Toolkit - a cross-platform GUI toolkit using Free Pascal
http://fpgui.sourceforge.net/

My public PGP key:  http://tinyurl.com/graeme-pgp
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

Michalis Kamburelis-3
In reply to this post by Michael Van Canneyt
>>  Right now we only have "with notifications, and not type-safe"
>> containers (TObjectList/TList) and "without notifications, and
>> type-safe using generics" containers (TFPObjectList/TFPList).
>
>
> These latter 2 are not generics based. There was some experimental code
> which attempted to implement TFPObjectList/TFPList on top of generics,
> but they were slower than the native implementation. That experiment is
> unmaintained.
>
> TFPGList is the one you meant, probably.
>

Ups. Yeah, by "without notifications, and type-safe using generics"
containers I meant TFPGList and TFPGObjectList (and other TFPGXxx)
from the FGL unit.

Regards,
Michalis
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

Sven Barth-2
In reply to this post by denisgolovan

Am 20.02.2016 11:33 schrieb "denisgolovan" <[hidden email]>:
>
> Hi
>  
> That always keeps me wondering - why neither FPC, no any other mainstream static-type language implements specialization based not only on types, but on compile-time constants as well?
>

Then you don't consider C++ as mainstream, do you?

> I mean something like
>  
>   TGenericList<T,options> = class
>     ...
>   end;
>  
> procedure TGenericList<T,options>.Delete(index:integer);
> begin
>   if (nnNotifyDelete in options) and Assigned(FOnDelete) then
>      FOnDelete(...);
>   end;
> end;
>  
> // usage
> type
>   TMyList = specialize TGenericList<integer, [nnNotifyAdd, nnNotifyDelete, nnNotifyChange]>;

I had thought about this some time ago and then found a reason why I wouldn't do it... Now if I'd only remember that reason -.-

Regards,
Sven


_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Addition of OnNotify event on "TFPGObjectList" and "TFPGList" triggered on the insert, delete and extract methods.

denisgolovan

21.02.2016, 02:59, "Sven Barth" <[hidden email]>:
> Then you don't consider C++ as mainstream, do you?
>
> Regards,
> Sven

Hm. Looks like C++ actually does that and that's great.
Unfortunately I stopped using C++ for large projects about 12 years ago, so my C++ template knowledge must be too old :)  

> I had thought about this some time ago and then found a reason why I wouldn't do it... Now if I'd only remember that reason -.-
Yes, it's interesting to see why. It's wildly useful and gives a lot of power.

--
Regards,
Denis Golovan
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal