kryo icon indicating copy to clipboard operation
kryo copied to clipboard

#894 Enable TaggedFieldSerializer to skip null fields (only works wit…

Open xuedonglxd opened this issue 2 years ago • 1 comments

In my scenario, we have some really big classes with nearly a thousand fields, but each time we would probably just read 20~30 fields, based on our purposes. So serializing and deserializing the whole entity with chunked encoding could be a serious waste from the performance perspective.

(Well, things are like this because those entities are really basic in our business, e.g video entity for Youtube; and their fields are managed by metadata systems, so they grow big really fast. But different fields are used by different businesses, and most like only a few fields at a time.)

I have done some tests, both the time consuming and the packet size are both considerably smaller then not skipping null fields. (and actually even for entities which are not so big ,the effect is quite obvious as well ,as long as there are null fields). I could share those test results if necessary. And I am not sure if I should add test cases as well for the new feature.

My solution is to filter the cached fields when trying to write the objects, and to do absolutely nothing when reading them.

I am not quite sure if it's a common case, if it's done in a proper way, or in a correct form (it's my very first PR on github). So just please take a look. Thanks.

xuedonglxd avatar Apr 29 '22 07:04 xuedonglxd

@xuedonglxd: Thanks for your PR!

The PR itself is good, but as I said in the ticket, I'm not sure this is a widely useful feature.

It would be possible to introduce a hook like protected getWriteTags(object) that can be used to filter the fields before writing them. Then you would only have to override a single method to get your custom behavior. I'm a bit reluctant to change to contract of TaggedFieldSerializer just for this use case.

For now, let's keep this PR open and see if anyone else is interested in this.

theigl avatar May 05 '22 10:05 theigl