Variable strings size #2044
Merged
Variable strings size #2044
Conversation
This file contains 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
fbs
approved these changes
Nov 6, 2021
Instead of using 64 bytes for all strings, use their actual size for literals. The size includes the terminating null character. This implements support for variable-sized strings for local vars by using the maximal size of a string assigned to a variable as the variable size.
Support for variable string size in map values. Maps must be always updated with same sized values, so we need to fill zeroes in case of updating it with a shorter string. This commit also removes SemanticAnalyser::update_assign_map_type, which didn't do anything useful and replaced it by update_string_size.
Support variable string size in both single- and multi-valued keys. Similarly to map values, when a string smaller than the key size is used, we fill the rest with zeroes. Semantic analysis for map keys has been changed a bit - map key updating starts in the 2nd pass as we need to wait until `map.skip_key_validation` is properly set in the first pass. This affects some of the semantic analyser tests.
This is done in a similar way to support for variable-sized simple strings, except it is more difficult to check if one tuple has all strings smaller than the other tuple. We add a new method SizedType::FitsInto to check this.
viktormalik
force-pushed
the
variable-str-size
branch
from
7acfa6d to
8037067
Compare
This was referenced Aug 4, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Instead of using 64 bytes for all strings, use the actual size for string literals. The size includes the terminating null character. When multiple different strings are assigned to a variable/map/map key, its size is determined by the largest assigned string. Strings with length unknown at compile time remain 64 bytes long.
This saves some space on the stack as string literals are usually short and also fixes issues coming from different string sizes, such as #2025.
Fixes #2025, also replaces and closes #2036.
Note: to simplify reviewing, I split implementation for variables, maps, and tuples into separate commits. However, tests do not pass after each commit (since the string literal size is changed in the first commit) so I can squash them together after the reviews, if you prefer.
Checklist
man/adoc/bpftrace.adocand if needed indocs/reference_guide.mdCHANGELOG.md