apple / swift-protobuf

Plugin and runtime library for using protobuf with Swift
Apache License 2.0
4.55k stars 443 forks source link

Have simple decode methods just return values #246

Closed tbkka closed 7 years ago

tbkka commented 7 years ago

As discussed in PR #243:

https://github.com/apple/swift-protobuf/pull/243/files/e397b41fa359a4aa7f6fec465b4a2d357b661d65#diff-2cc7a2fc1136ffc3ff613b97a60aa72cR63

We should be able to simplify the Decoder protocol by having the simple decode methods (singular fields of primitive types) return a value instead of accepting the field as an inout parameter. For example, the current

decodeSingularInt32Field(value: inout Int32?)
decodeSingularInt32Field(value: inout Int32)

would collapse to a single

decodeSingularInt32Field() -> Int32?

The discussion referenced above covers some of the touchy points:

thomasvl commented 7 years ago

@tbkka should this be in the milestone also?

tbkka commented 7 years ago

Looks like this change makes 100 int64 about 15% slower -- apparently returning an Optional is measurably more expensive than assigning to an inout parameter. (This holds for both proto2 and proto3, interestingly.)

Based on that, I think I'll leave the code with separate decode methods for optional and non-optional field types.

tbkka commented 7 years ago

I'm going to close this -- the current code is a little more verbose, but it's consistent and a little faster so I think it's preferable.