New patch reinstates the three-way check, but puts it in a separate
static method. Performance seems to be just fine.
I wonder if my previous round of benchmarking with the inline
three-way check was somehow flawed... Either that or the JIT is
happier with the new arrangement for some reason. In any case I kind
of like the separate isAlien.
On 13 May 2014 14:47, Michał Marczyk <michal.marc...@gmail.com> wrote:
> Unfortunately the Guava example means that this approach doesn't
> really solve the problem at hand... Thanks for pointing it out.
>
> The earlier version with the three-way test in place of the
> "java.util." check should be semantically correct, at least.
>
>
> On 13 May 2014 14:40, Michał Marczyk <michal.marc...@gmail.com> wrote:
>> Also, = defers to equals in all cases except two: (1) numeric
>> arguments, (2) at least one IPersistentCollection among the arguments.
>>
>> Clojure collections are allowed to determine whether they are = to the
>> other thing or not. So, we should calculate special hashCodes for the
>> java.util collections Clojure collections know about.
>>
>> Other collection types will just use equals for comparisons and so for
>> them we have to return hashCode to preserve =/hash semantics by
>> relying on the equals/hashCode contract -- there is nothing else we
>> can do. Well, strictly speaking we could apply an arbitrary function
>> to hashCode, so murmuring it (as with Strings) is an option; that
>> would come at a performance cost, however (I benchmarked a version
>> that did that), and I don't think there'd be much benefit -- certainly
>> none at all for classes using identity hash codes.
>>
>> Cheers,
>> Michał
>>
>>
>> On 13 May 2014 14:20, Michał Marczyk <michal.marc...@gmail.com> wrote:
>>> It's not just to facilitate inlining, but also to limit the perf hit
>>> for hashing non-collections, some of which make completely reasonable
>>> map keys and set members. I've used the classes Alex Miller mentioned
>>> he was interested in for benchmarking: Class, Character, Var; these
>>> are all good examples, and only one is under Clojure's control.
>>>
>>> I used a "map or map entry or collection" test in earlier versions of
>>> the patch, but switched to the class name hack because the three-way
>>> type check seems to slow down getting to the default case much more
>>> than examining the class name. For types that actually implement
>>> IHashEq, however, the original patch was faster, so I think HotSpot is
>>> happy to inline either version.
>>>
>>>
>>> On 13 May 2014 14:00, Mike Fikes <mikefi...@me.com> wrote:
>>>> To facilitate inlining, the patch calls out to a separate larger method
>>>> which handles a group of cases.
>>>>
>>>> + if(o.getClass().getName().startsWith("java.util."))
>>>> + return doalienhasheq(o);
>>>> + return o.hashCode();
>>>>
>>>>
>>>> I was wondering whether an efficient improvement is possible that would
>>>> support things like Guava ImmutableList.
>>>>
>>>>
>>>> In particular, I was wonder which "default" cases are currently handled by
>>>> the return o.hashCode() above. Replacing the three lines above with
>>>>
>>>>
>>>> + return doalienhasheq(o);
>>>>
>>>>
>>>> would allow the patch to also handle non-java.util collection
>>>> implementations, but push the "default" cases down into the bottom of that
>>>> method.
>>>>
>>>>
>>>> On Tuesday, May 13, 2014 12:38:54 AM UTC-4, Michał Marczyk wrote:
>>>>>
>>>>> I've posted a patch that makes java.util.{List,Map,Map.Entry,Set}
>>>>> hashes consistent with those of appropriate Clojure collection types
>>>>> on the ticket.
>>>>
>>>> --
>>>> You received this message because you are subscribed to the Google
>>>> Groups "Clojure" group.
>>>> To post to this group, send email to clojure@googlegroups.com
>>>> Note that posts from new members are moderated - please be patient with
>>>> your
>>>> first post.
>>>> To unsubscribe from this group, send email to
>>>> clojure+unsubscr...@googlegroups.com
>>>> For more options, visit this group at
>>>> http://groups.google.com/group/clojure?hl=en
>>>> ---
>>>> You received this message because you are subscribed to the Google Groups
>>>> "Clojure" group.
>>>> To unsubscribe from this group and stop receiving emails from it, send an
>>>> email to clojure+unsubscr...@googlegroups.com.
>>>>
>>>> For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clojure@googlegroups.com
Note that posts from new members are moderated - please be patient with your
first post.
To unsubscribe from this group, send email to
clojure+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups
"Clojure" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to clojure+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
