Swift: encode & decode size-delimited messages

[jszumski]

Adds the ability to encode and decode a size-delimited message collection in Swift.

The intended use case is to decode a stream of messages recorded to disk (e.g., Bazel's Build Event Protocol). Similar implementations exist in Java (writeDelimitedTo and parseDelimitedFrom) and other languages in protocolbuffers/protobuf#10229.

[jszumski]

Should this calculation attempt to reserve some capacity for the varints or just let the buffer expand naturally?

[efirestone]

Better to allocate slightly more than slightly less (expanding requires copying memory). I would change to (8 + structSize) * values.count where the 8 is 64 bits for the varint.

There's probably already plenty of buffer realistically, since I think the struct size is almost always larger than what is written, but I'd still go with what I mentioned and we can do some profiling and optimizing at a later date.

[jszumski]

Updated to reserve space 👍

[jszumski]

This was required to ensure storage was allocated in all cases (see testAppendEmptyFirst)

[efirestone]

I'm a little wary of calling malloc(0) in expand(). In C at least, the behavior for this can vary from runtime to runtime.

Can we achieve the same thing by adding something like this into append:

guard !data.isEmpty else { return }

[jszumski]

Added guards where appropriate

[efirestone]

Does the spec specify that "no bytes" should be one empty object vs. an empty array? (Or can we verify with protoc?)

[jszumski]

This should have been an empty array

[efirestone]

Better to allocate slightly more than slightly less (expanding requires copying memory). I would change to (8 + structSize) * values.count where the 8 is 64 bits for the varint.

There's probably already plenty of buffer realistically, since I think the struct size is almost always larger than what is written, but I'd still go with what I mentioned and we can do some profiling and optimizing at a later date.

[efirestone]

FYI: We have some tricks inside the writer to avoid this write-then-copy inside of the ProtoWriter (see beginLengthDelimitedEncode). They're probably not worth doing here, but for future archaeologists who might be trying to optimize this I'll mention.

Basically, most messages will be in the size range that requires (IIRC) two bytes of varint, so we reserve two bytes, then write out value into the full buffer. If it happens to not take two bytes for its size, only then do we go back and move it to add more or less room for the prefix size.

In the future, we could extract this into an extension on WriteBuffer, like:

// This will reserve the length bytes, then write the buffer, then adjust and fill in the length
// bytes based on what was written.
buffer.writeLengthEncoded { tempBuffer in
   let writer = ProtoWriter(data: tempBuffer, ...)
   try value.encode(to: writer)
}

Again, all a good future enhancement. Not required now.

[efirestone]

I'm a little wary of calling malloc(0) in expand(). In C at least, the behavior for this can vary from runtime to runtime.

Can we achieve the same thing by adding something like this into append:

guard !data.isEmpty else { return }

[oldergod]

Could you make sure all your comments are sentences?, they all look good but some are missing a period.