-
Notifications
You must be signed in to change notification settings - Fork 1
Allocationless parsing #30
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
Merged
Merged
Conversation
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
…atch the signature of main()
…ediate storage of arguments during parsing
clang-tidy review says "All clean, LGTM! 👍" |
clang-tidy review says "All clean, LGTM! 👍" |
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.
A major goal of this library is to minimize the amount of heap allocations that are done during parsing. Previously, all arguments passed to the parse method of the Parser were inserted into a std::vector. One by one, the front element would be popped off and processed. Though this solution was simple, it was expensive due to two main reasons. First, we allocated a vector to store the elements, which wasn't necessary since we already received the elements in a container—a raw C-string array. Second, erasing elements from the front multiple times is costly, as it requires moving the remaining elements one by one every time.
In this development branch, I have removed the usage of std::vector and replaced it with a view. I use the
std::views::counted
function, which, when used with a raw array of C-strings, returns astd::span
. This change allows us to apply modern container access practices with the plain old array. Instead of removing elements from the original array, I can now shrink the view in each iteration until the view is empty.The main difference in this branch can be seen by comparing simplified versions of the two implementations.
The old version is as follows:
In contrast, the new version is:
Before deciding on this approach, I experimented with a custom container that used std::span internally. While it initially seemed to work, it became unnecessarily complex for my needs. I retained some utility functions I had already written for testing, as they will likely be useful in the future.
This optimization should lead to significant performance improvements, particularly in scenarios involving a large number of arguments.