-
Notifications
You must be signed in to change notification settings - Fork 478
Add parameter to control extension output #1795
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
Conversation
We're definitely interested in reducing the amount of codegen, but making it based on options is problematic: beyond just introducing more complexity, it also requires that all protos used throughout the entire system be generated with the same options to work correctly. I think we're better off exploring ways of universally reducing codegen without removing features, such as the bytecode-based approach I've started looking at in #1789. |
You might also want to read some of the past things that have come into play looking at binary size.
|
Thanks for the feedback - I completely understand the hesitation about introducing new option-driven divergence in codegen. So by the default the generated code will be the same. Universally reducing codegen is a great north star. The ultimate version of reducing the code - is to simply not include it to the binary. That's why I chose this direction. 100% agree that with great power comes great responsibility. But at a stage when developers care about the app size, they definitely can handle this power. Could you explain in more details why generating code based on options is problematic? |
The problems can come when two different things both need the same set of protos. With fewer options, they can share that generation and thus the final binary only needs one copy. But if options change the generation, they can need things generated differently. This can result in them not being able to share a common generation and have to do module specific copies. But worse, if also means the generated proto types likely must be implementation details, i.e. - they can't put the messages in their public interfaces as that is more likely to result in the generation choices having problem if the protos are needed by something else. |
btw - I haven't really looked at them, but SwiftPM Package Traits might be something to also look at why trying to think about this problem. I'm not sure that it would directly be usable, but since it seems like it might be a useful concept to reference on these topics. |
And when the generated code is slit into several files, two different things can grab whatever files they need. Is this correct? |
As long as the files aren't exclusive, yes. This PRs ideas around "debug only" could still be a problem from that pov. If one module always needs something and another only wants it in debug, that doesn't seem like it could easily be resolved. |
Let's forget about the debug only 🙂 If I have Here is a draft with possible implementation - #1796 |
It could be. As with the referenced other ideas, I don't think anyone had found something we were sure was "right", vs. exploring options. We did know generation options did have some downsides when used in larger/complex proto graphs. |
Hey, guys 👋
Code generation can significantly bloat the app size in large projects. The ability to fine-tune what is generated will help minimize impact while still allowing to enable features when they are needed.
This PR just shows the general direction, and I'd love to hear your thoughts so we're aligned 🙂
What has changed:
_MessageImplementationBase
is replaced with_MessageEquatable
and_MessageHashable
Before:
After:
For example we have proto:
Here are some examples of how GenerateExtensions changes code generation: