Fix big performance issue in string serialization#1848
Merged
alamb merged 3 commits intoapache:mainfrom May 13, 2025
Merged
Conversation
The previous implementation wrote each character individually using write! macro, which is inefficient for string formatting. The new implementation uses write_str to write larger chunks of the string at once, significantly reducing the number of write operations and formatting overhead. This change maintains the same escaping behavior but improves performance by avoiding character-by-character writes.
…splay implementation
jayzhan211
approved these changes
May 12, 2025
Contributor
|
When you say "old" code do you know what PR introduced this regression? |
Contributor
Author
|
@alamb : I meant that the code before this PR handled the string char by char. I don't think this was a regression. |
Contributor
|
Thanks @lovasoa and @jayzhan211 for the review cc @iffyio |
Contributor
Author
|
thanks for merging ! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The old code was handling string escaping character by character. For every literal string in the AST, it would push it to the underlying writer character by character, resulting in thousands of write calls for long strings.
The new code calls the write function only once, with the entire string, in most cases.
Only when the string is stored unescaped do we really need to call write multiple times; and even then, we don't need to call it more than the total number of characters to escape plus one.
Here are benchmark results for serializing the following sql statement:
"SELECT 'xxx...(x 10000)' as long_string"to a string in memory: