[fpc-devel] Attn. Generics.Collections and rev. 39345 and speed slowdown

[Maciej Izak]

Hi,

renaming for TMormotHashFactory to TGenericsHashFactory is somehow
acceptable (but IMO bad taste) but change for default hashing algorithm in
such important library is really really really bad idea.

This was not consulted, change like that or proposition should be reported
here :

https://github.com/maciej-izak/generics.collections/issues

OR IN ANY OTHER WAY, not in comment in other bug report where can be easily
omitted (!).
The problem with thashmapextendedequalitycomparer example was really minor
and should be fixed in other way not by changing *default hashing
algorithm* for whole library (!).

Widely used TDictionary<TKey, TValue> is also affected by this change.

Results BEFORE Michael Van Canneyt changes:

THashSet<string> add 2000000 items. Time elapsed : 1844
THashSet<string> lookup 2000000 items. Time elapsed : 812
TSortedHashSet<string> add 2000000 items. Time elapsed : 18640
TSortedHashSet<string> lookup 2000000 items. Time elapsed : 641

THashSet<integer> add 10000000 items. Time elapsed : 1328
THashSet<integer> lookup 10000000 items. Time elapsed : 640
TSortedHashSet<integer> add 10000000 items. Time elapsed : 4235
TSortedHashSet<integer> lookup 10000000 items. Time elapsed : 765

Results AFTER Michael Van Canneyt changes:

THashSet<string> add 2000000 items. Time elapsed : 2250
THashSet<string> lookup 2000000 items. Time elapsed : 906
TSortedHashSet<string> add 2000000 items. Time elapsed : 18828
TSortedHashSet<string> lookup 2000000 items. Time elapsed : 688

THashSet<integer> add 10000000 items. Time elapsed : 2219
THashSet<integer> lookup 10000000 items. Time elapsed : 1140
TSortedHashSet<integer> add 10000000 items. Time elapsed : 5250
TSortedHashSet<integer> lookup 10000000 items. Time elapsed : 1234

[Michael Van Canneyt]

Post by Maciej Izak
Hi,
renaming for TMormotHashFactory to TGenericsHashFactory is somehow
acceptable (but IMO bad taste) but change for default hashing algorithm in
such important library is really really really bad idea.

As noted in the bugreport, your own examples did not compile with your supplied code.
Next time, please test that too.

It is definitely better to have a slower default that works always than a faster one
that does not let all code compile.

But not problem, please advise on the changes needed to
a) fix the mentioned example so it actually compiles.
b) Speed up the default hash.

I (or someone else) will be happy to change it.

Michael.
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/

[Maciej Izak]

Post by Michael Van Canneyt
As noted in the bugreport, your own examples did not compile with your supplied code.
Next time, please test that too.

The most important was to finish new collections, deep refactoring and
proper tests suite than testing all examples. Library was totally
refactored and something like that may happens.

Post by Michael Van Canneyt
It is definitely better to have a slower default that works always than a
faster one that does not let all code compile.

No. You are wrong. The more important is test suite than minor example
somewhere (which can be easily fixed). For sure faster library with proper
test suite is more important than a little outdated example (!sic).

Post by Michael Van Canneyt
But not problem, please advise on the changes needed to a) fix the
mentioned example so it actually compiles.
b) Speed up the default hash.
I (or someone else) will be happy to change it.

Ok, but in the future you should ask for such important changes.

[Michael Van Canneyt]

Post by Maciej Izak

Post by Michael Van Canneyt
But not problem, please advise on the changes needed to a) fix the
mentioned example so it actually compiles.
b) Speed up the default hash.
I (or someone else) will be happy to change it.

Ok, but in the future you should ask for such important changes.

I could do so and I will.
I consider the matter unimportant.

We can argue about this for a long time.
We all have restricted time, so please just provide a correct patch.

Michael.
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepasc

[Sven Barth via fpc-devel]

Post by Maciej Izak
Hi,
renaming for TMormotHashFactory to TGenericsHashFactory is somehow
acceptable (but IMO bad taste) but change for default hashing algorithm in
such important library is really really really bad idea.

No, the bad taste is too have a class that appears to be named specifically
to fit a specific third party project inside a general purpose library.

(though I agree that the change of the default hash could have been
discussed first)

Regards,
Sven

[Maciej Izak]

2018-06-30 8:55 GMT+02:00 Sven Barth via fpc-devel <

Post by Sven Barth via fpc-devel
No, the bad taste is too have a class that appears to be named
specifically to fit a specific third party project inside a general purpose
library.

Ok, I agree here, but why Michael Van Canneyt decide to annihilating only
TmORMotHashFactory and TDelphi*HashFactory still exist? This is highly not
consequent!

Post by Sven Barth via fpc-devel
(though I agree that the change of the default hash could have been
discussed first)

seems that compilation for minor example (easy fixable in other way) has
bigger priority than major performance of whole library :P

[Sven Barth via fpc-devel]

Post by Maciej Izak
2018-06-30 8:55 GMT+02:00 Sven Barth via fpc-devel
No, the bad taste is too have a class that appears to be named
specifically to fit a specific third party project inside a
general purpose library.
Ok, I agree here, but why Michael Van Canneyt decide to annihilating
only TmORMotHashFactory and TDelphi*HashFactory still exist? This is
highly not consequent!

mORMot is a third party project that can be used with FPC while Delphi
is essentially a template and base line for compatibility, so there is a
difference between the two. Though if you have a formal name for the
hash that Delphi uses we can change that as well :P

Regards,
Sven

[Maciej Izak]

2018-06-30 11:03 GMT+02:00 Sven Barth via fpc-devel <

mORMot is a third party project that can be used with FPC while Delphi is
essentially a template and base line for compatibility, so there is a
difference between the two. Though if you have a formal name for the hash
that Delphi uses we can change that as well :P

TDelphi*HashFactory is not required for compatibility with Delphi (as many
other things in rtl-generic). Correct name for this hash is "BobJenkins"
(Delphi has special slightly modified version of HashLittle which in Delphi
is called BobJenkins - don't ask :P). I will fix all hash names where
"Delphi" word exist.

[Michael Van Canneyt]

Post by Maciej Izak
2018-06-30 8:55 GMT+02:00 Sven Barth via fpc-devel <

Post by Sven Barth via fpc-devel
No, the bad taste is too have a class that appears to be named
specifically to fit a specific third party project inside a general purpose
library.

Ok, I agree here, but why Michael Van Canneyt decide to annihilating only
TmORMotHashFactory and TDelphi*HashFactory still exist? This is highly not
consequent!

Because maybe TDelphi*HashFactory is needed for Delphi compatibility.
I did not have time to investigate that.

We do not aspire to compatibility with mORMot, this is for sure, and
therefor I renamed it. Feel free to suggest another name.

Michael.
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mai