Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove o == this check from generated equals? #1633

Open
perceptron8 opened this issue Nov 9, 2023 · 2 comments
Open

Remove o == this check from generated equals? #1633

perceptron8 opened this issue Nov 9, 2023 · 2 comments

Comments

@perceptron8
Copy link
Contributor

It appears that most cases o == this check actually degrades performance because of branch prediction algorithms. It might be worth considering to remove it from auto-value templates. Sorry for linking to YT, but there's no article anywhere - at least I couldn't find any.

Please see https://www.youtube.com/watch?v=kuzjX_efuDs.

@eamonnmcmanus
Copy link
Member

Thanks for the pointer! I watched the video, and I admit I was confused for a while about exactly what the "Guava generator" was that is referred to. I think it is actually the Guava Eclipse Plugin, which is not produced by the team responsible for Guava, though AutoValue is.

It's certainly interesting to wonder whether the == check is worthwhile. The measurements in the video are maybe a bit misleading, because comparing two int values with == is very fast. Where the instance check may be more helpful is if there are several properties that have their own equals methods. Including String properties, for example. Then if you do end up comparing an object with itself, without an instance check you have to call equals on all the properties before concluding that, yes, it's equal.

I think most of the time when you care about the performance of equals on AutoValue objects it is because they are in a HashSet or keys in a HashMap. I'm going to handwave and say that usually you'll either never be comparing an object against itself (you construct an object and then see whether it is already in a set or map) or fairly often (you have a canonical list of objects and some of them are in a set or map). In the first case, the branch prediction argument suggests that the o == this check will be cheap because we will end up predicting it is false and we will be right. In the second case, it depends on the balance between the cost of the o == this check (which might incur a prediction penalty), how often it is true, and how expensive it is to compare all the properties instead.

Still, it might be worth thinking about some sort of heuristic. For example, only do the == check if there is more than one property that is an object rather than a primitive.

@cpovirk
Copy link
Member

cpovirk commented Nov 10, 2023

I have idly wondered before whether AutoValue, protobuf, etc. might be able to optimize the order in which they compare fields, like by comparing all primitives before any objects. Really, the ideal is probably for the code author to design the class such that the fields that are most likely to differ come first (though that's a bit of an odd thing to encode into the API :))... or maybe for profile-guided optimization to pick the ordering. But that's hard when the gains are diffuse—and maybe small even in total.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants