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

rpc: optimize small tuple deserialization #2519

Closed
wants to merge 1 commit into from

Conversation

avikivity
Copy link
Member

When rpc deserializes a tuple, it cannot do so into a std::tuple constructor arguments, since argument evaluation order is unspecified but deserialization must proceed left-to-right. So we deserialize after the tuple is constructed.

However, if the tuple has zero or one elements, then there is no reordering problem. Take advantage of that and deserialize in the constructor, saving some steps.

I observed a significant reduction in text size in ScyllaDB's messaging_service.o:

text data bss dec hex filename
6797398 48 236 6797682 67b972 messaging_service.o.before
6758146 48 236 6758430 67201e messaging_service.o.after

A 40kB reduction.

When rpc deserializes a tuple, it cannot do so into a std::tuple
constructor arguments, since argument evaluation order is unspecified
but deserialization must proceed left-to-right. So we deserialize after
the tuple is constructed.

However, if the tuple has zero or one elements, then there is no
reordering problem. Take advantage of that and deserialize in the
constructor, saving some steps.

I observed a significant reduction in text size in ScyllaDB's
messaging_service.o:

   text	   data	    bss	    dec	    hex	filename
6797398	     48	    236	6797682	 67b972	messaging_service.o.before
6758146	     48	    236	6758430	 67201e	messaging_service.o.after

A 40kB reduction.
@avikivity
Copy link
Member Author

@xemul please review

@xemul xemul closed this in ab6cde5 Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants