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

Add option to skip Hashable/Equatable conformances #1476

Open
rockbruno opened this issue Oct 12, 2023 · 6 comments
Open

Add option to skip Hashable/Equatable conformances #1476

rockbruno opened this issue Oct 12, 2023 · 6 comments

Comments

@rockbruno
Copy link

Hello!

We make extensive usage of SwiftProtobuf in our app and we find it to be responsible for a great chunk of our app's total install size. I see we already have a thread for more general potential size improvements, but I'd like to talk about the Hashable conformance specifically.

SwiftProtobuf is generating Hashable/Equatable conformances for all protos by default, but upon closer inspection our app only truly required those capabilities for two or three of our protos. I forked the compiler and tried to see what would happen if I disabled conformances for everything except for those protos, and the result was an install size reduction of 1MB.

It seemed to be that it could be possible to add a new flag to compiler to indicate that it should "skip" generating those conformances, but I didn't go very far because I was not sure what to do with some of the built-in types like the AnyMessage erasure. I thought then that this would be worth a conversation. What do you think of the idea of not generating conformances if we don't need them, for the purposes of reducing app size?

@thomasvl
Copy link
Collaborator

My initial thought is this sorta seems like maybe it could be considered in the same way the TextFormat/JSON support could be, i.e. - generate Equatable/Hashable into a different source file, so folks can pick up the dependencies they want.

But generally this has the same problem as those other discussions, when someone ends up with overlap in the protos they need between two subproject and those projects have picked different options, there's no go way to handle things. 😦 (Maybe the multiple sources would leads to different build targets for each so the dependencies could pull in the transitive dependency requirements when needed to avoid the conflict you get with options.)

@tbkka
Copy link
Collaborator

tbkka commented Oct 19, 2023

Splitting Hashable/Equatable conformance declarations into separate source files seems to me like the first approach we should try here. It's not always easy for build systems to ignore and/or delete unwanted generated sources, so we may be forced to consider other approaches someday.

The issue @thomasvl outlined is a concern for framework authors but I'm personally happy to let the individual framework authors deal with that issue as they see fit.

@tbkka
Copy link
Collaborator

tbkka commented Oct 19, 2023

(P.S. Splitting out sources is intrusive enough that we should try to squeeze that into 2.0 if we can.)

@cburrows
Copy link
Contributor

I am surprised to hear that Hashable conformance causes a headache, because I thought we implemented it in the library using the message visitor. What is the impact on binary size for hash(into:) on generated types? Something much > 0?

But we do generate an Equatable conformance in terms of the generated fields, right? I wonder how much space we could get back if we could cleverly push most of that functionality up into the library, as Hashable, so as to generate no func == or a small one.

In other words maybe it's feasible to try to tackle this as a performance problem head on first? Before adding an option to side-step it?

Also, forgive my ignorance but does the compiler have a pass that ensures it emits functions with identical implementations at most once? If we can assume that runs we might be able to push farther, too.

@tbkka
Copy link
Collaborator

tbkka commented Oct 19, 2023

I am surprised to hear that Hashable conformance causes a headache, because I thought we implemented it in the library using the message visitor.

Good point. We do implement it as a visitor and so the overhead should be minimal.

But we do generate an Equatable conformance in terms of the generated fields, right? I wonder how much space we could get back if we could cleverly push most of that functionality up into the library, as Hashable, so as to generate no func == or a small one.

The tricky part is simultaneously visiting two messages at the same time. It's not obvious to me how to do this, but I'm sure it's possible. Hmmm....

Also, forgive my ignorance but does the compiler have a pass that ensures it emits functions with identical implementations at most once?

There is an optimization pass called "function merging" that attempts to identify identical functions and combine them. I'm not sure how effective it is in these cases.

@tbkka
Copy link
Collaborator

tbkka commented Oct 19, 2023

I am surprised to hear that Hashable conformance causes a headache, because I thought we implemented it in the library using the message visitor.

To be clear, we implement the hash(into:) method in this way. The Hashable conformance also requires ==, which is currently directly generated.

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

No branches or pull requests

4 participants