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

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xuedonglxd
Copy link

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.

@theigl
Copy link
Collaborator

theigl commented May 5, 2022

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants