groovy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jochen Theodorou <blackd...@gmx.org>
Subject Re: Towards a better compiler
Date Mon, 24 Apr 2017 13:46:04 GMT


On 24.04.2017 10:56, Cédric Champeau wrote:
>
>
>     Implementing equals/hashcode does not give you any order in a Set or
>     Map you can rely on. Really, I still don´t get what you target with
>     that. You require them in set to avoid duplicates.
>
>
> I'm not talking about order here. I'm talking about reproducibility. A
> linked hash set guarantees that things are read in the order they were
> inserted. This is in general just enough. For example, we read in the
> bytecode a list of interfaces, we add them to a set. If it's not linked,
> then when we read them back, you have no guarantee. It has nothing to do
> with _sorting_, which would reorder interfaces typically. The 2 uses
> cases are different.

So we agree it is the "linked" ability that gives the order and equals 
and hashcode are secondary

>     then don´t. Let´s change the API to not return Set.
>
> Yes. I would say +100 if it wasn't for performance. Typically if the
> algorithm needs to use  `contains`, a `Set` is way more efficient.

I was saying to use List, because there is no equals and hashcode 
defined. If they are not defined your search will be invalid unless, you 
are searching using the exact same instance.

And... what hashcode function do you suggest for MethodNode and 
ClassNode? Because I think they won't be cheap. If they are too 
expensive "way more efficient" will become very different for small sets.

> And
> that's exactly why we use sets in lots of places. So to be clear, I did
> a quick review of where we used sets and maps, and replaced that with
> linked hash set and linked hash map when we needed both to guarantee
> that the order of insertion is preserved (not an absolute order) *and*
> that the algorithm required fast checks. This is all we need, I think.
> But I'm calling for more than just a quick review, we should recheck all
> our usage precisely. And avoid binary breaking changes, of course :)


as I said, maps are probably different, since there we most probably do 
not use the AST instances as key, and if we did, it is probably a bug. 
For the set cases.... well, maybe it would be better to give the 
container better search abilities instead of exposing a special 
collection here. That way we can ensure we have the implementation we need.

bye Jochen

Mime
View raw message