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

[Proposal] Code size reduction for generated traverse methods #1500

Open
mjburghard opened this issue Nov 10, 2023 · 2 comments
Open

[Proposal] Code size reduction for generated traverse methods #1500

mjburghard opened this issue Nov 10, 2023 · 2 comments

Comments

@mjburghard
Copy link
Contributor

mjburghard commented Nov 10, 2023

We updated from version 1.17.0 to 1.20.3 and saw some large code size additons.
We tracked them down to #1042 and #1183. After some thinking, I came up with an idea to reduce the generated code:

Currently, depending on the type of a field, code like the following is generated:

try { if let v = _storage._singularInt32 {
   try visitor.visitSingularInt32Field(value: v, fieldNumber: 1)
} }()

Through the introduction of an helper function on Vistor that performs the optional unwrapping, we could generate the following code instead:

try {
   try visitor.visitSingularInt32Field(optionalValue: _storage._singularInt32, fieldNumber: 1)
}()

The extension for Vistor looks like this:

extension Visitor {

    mutating func visitSingularInt32Field(optionalValue value: Int32?, fieldNumber: Int) throws {
        if let value {
            try visitSingularInt32Field(value: value, fieldNumber: fieldNumber)
        }
    }
} 

This would move the optional checks out of the traverse method and its closures, but it requires one additional call to the helper function.
I did some naive tests of this apporach on compiler explorer and it seems to reduce the generated machine code for my simplified example.

I am happy to contribute an implementation, but wanted to propose and discuss the change first.
I will also verify that there is a code size reduction for optimized builds as well.

(The approach should work on the 1.x branch as well as main.)

@thomasvl
Copy link
Collaborator

Just confirming first, you are talking about size changes in release builds? How much of a size change for how many/how complex of messages?

There's also some discussion going on on #861 about changing things to support a new behavior that also has the potential to change code sizes of things.

@mjburghard
Copy link
Contributor Author

Yes, a size change in a release build. We went up by 410.38 kB and have about 1400 messages.

Thanks for the other thread.

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

2 participants