ruby-protobuf / protobuf

A pure ruby implementation of Google's Protocol Buffers
https://github.com/ruby-protobuf
MIT License
463 stars 101 forks source link

Speed Up Creates & Updates By Memoizing on encode #293

Closed vintagepenguin closed 8 years ago

vintagepenguin commented 8 years ago

Serialization is one of the slowest things that you do in Protobuf. If we already have an encoded message, then we don't need to encode it again so why not memoize it.

@abrandoned @liveh2o @mmmries @brianstien

vintagepenguin commented 8 years ago

@film42 commenting here since the diff you commented on got deleted.

But yeah, I think @encode = nil it is confusing if you're looking at this for sure.

But the goal here it to clear out that instance variable so that when we hit the memoization in the encode method we know that we need to encode the message again because there has been a change and we know that because the setter was called.

film42 commented 8 years ago

@vintagepenguin It would be more clear set a flag, or call a bust_encode_cache! method or something. But this seems :+1: .

Looks like you have a :cop: to fix, but then :shipit: .

claygoddard commented 8 years ago

It seems that with this change, performing operations on repeated fields is no longer handled properly after the memoized encoding has been computed.

a.encode
a.some_repeated_field << "new-value"
a.encode # doesn't contain "new-value"
liveh2o commented 8 years ago

That's definitely a problem, @goddardc. Mind writing up an issue?

claygoddard commented 8 years ago

@liveh2o Sure thing!