[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Making 'eq' == 'eql' in bignum branch

From: Paul Eggert
Subject: Making 'eq' == 'eql' in bignum branch
Date: Fri, 27 Jul 2018 14:14:59 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1

Ten days ago we were discussing the possibility of changing eq to be equivalent to eql in the bignum branch, to avoid possible compability issues where 'eq' no longer agrees with 'eql' (or with '=') on integer arguments. On July 18 Stefan wrote:

Some uses of EQ admittedly don't need to be
redirected to EQL, so we could improve the patch to reduce its cost, but
I expect the benefit would be small.

It's more than "some", if we count static occurrences. Almost all uses of EQ in C code don't need to be redirected to EQL, because one can easily tell statically that at least one of the arguments must be a non-float, or that it's OK to use pointer comparison for some other reason. I wrote a patch along those lines (see attached; it's against the master branch), and when compiled with normal -O2 optimization (Fedora 28 x86-64, AMD Phenom II X4 910e, user+system time, average of 3 runs), "cd lisp; make compile-always" suffers only a 2.3% slowdown, better than the 4% slowdown Stefan mentioned with his simpler patch.

Stefan, have you thought about hashing floating-point objects instead, so that comparing pointers suffices for eql, and eq becomes equivalent to eql in a different way? That may well have better performance for typical Emacs applications, since they typically don't create a lot of floating-point numbers. Also, it'd be less error-prone than the attached patch, which involves static analysis that would typically be done by hand.

>> Also, the C code will need to change how hashing works since XHASH
>> etc. must be consistent with eq.
> The patch does that already, AFAIK.

I still see a problem there in your simpler patch, since cmpfn_eql isn't used in the eq case, when hashing floating-point values.

>> I worry that the benchmark isn't realistic enough, as some usage of EQ in
>> C code will need to change to Feql or equivalent.
> I don't understand what you mean: the patch changes `EQ` itself:
>      -# define EQ(x, y) lisp_h_EQ (x, y)
>      +# define EQ(x, y) EQL (x, y)
> where EQL is the same as Feql (except it returns a boolean instead of
> a Lisp_Object).

You're right. Sorry, I missed that.

Attachment: 0001-Make-eq-act-like-eql.patch
Description: Text Data

reply via email to

[Prev in Thread] Current Thread [Next in Thread]