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

Micro-optimize command-to-RESP #34

Merged

Conversation

zuiderkwast
Copy link
Collaborator

This avoids a few nested lists, thus a few allocations.

Copy link
Collaborator

@bjosv bjosv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had some figures for this change by running them in a test run, do you have them? Just for the record.

@zuiderkwast
Copy link
Collaborator Author

zuiderkwast commented Sep 19, 2023

It's hard to get stable figures, but here is my best try.

Each test run encodes a large pipeline of commands (7 commands of 4821 bytes in
total) one million times. The times are nanoseconds per encode.

Run With this PR main branch Improvement %
1 7319 8100 9.64
2 6962 8221 15.31
3 8988 8400 -7.00
4 8144 8091 -0.65
5 8279 8094 -2.28
6 8092 8481 4.58
7 7397 7892 6.27
8 7039 7832 10.12
9 7464 7645 2.36
10 8105 8114 0.11
Avg 7778.9 8087 3.81

It's easier to motivate it theoretically.

Each nested list which is replaced by its elements inlined in the outer list saves one cons cell, i.e. one allocation of 2 words (16 bytes). So with fewer allocations, it's also less work for the garbage collector.

Example: [... , $\r, $\n, ...] instead of [... , "\r\n" , ... ] saves one cons cell.

@zuiderkwast zuiderkwast merged commit 87832d6 into Ericsson:main Sep 19, 2023
5 checks passed
@zuiderkwast zuiderkwast deleted the micro-optimize-command-to-resp branch September 19, 2023 11:47
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.

None yet

2 participants