GeezerVison is showing github.com — added `TokenList::Stream` class to wrap `std::istream` usage and implemented alternative C I/O version by firewave · Pull Request #244 · cppcheck-opensource/simplecpp
Conversation
firewave
commented
Mar 7, 2022
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Reading the file via std::istream includes a considerable overhead caused by std::istream::sentry and others. Using C I/O instead reduces the "total Ir" usage by about 10%.
Testing with -q -Ilib/ -D__GNUC__ lib/valueflow.cpp:
Clang 13 271,013,439 -> 269,398,283 -> 244,353,123
GCC 11 -> -> ``
The intermediate value shows the improvement if we only change the actual input file. The last value if we also do it for all the includes which does not require you to use the new interface function. So we even if you do not change the application which uses this it would still result in a sizable improvement.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
firewave
commented
Mar 7, 2022
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
Still needs the unit tests to be adjusted to test both implementations as well as making it properly selectable in the standalone binary.
This can possibly be introduced further by adding buffering to FileStream.
We could also add a MemStream version which works on a memory buffer allowing for the use case of just a passing a std::istringstream (i.e. string) and deprecating/removing the old API with std::istream.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
firewave
commented
Apr 19, 2022
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
Somehow using fgetc() seems to already interpret the characters causing 0x00 to be omitted when read even though the file is open in "binary" mode. Using fread() (which is supposed to be faster) fixes this but it is much slower.
Turns out it's just a bug in my code with unget() and multi-byte characters.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
firewave
commented
Oct 27, 2022
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
I backed out the test changes and will merge them into #261 so they don't get lost.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
firewave
commented
Oct 27, 2022
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
Testing with -q -Ilib/ -D__GNUC__ lib/valueflow.cpp:
current -> std::istream input -> FILE* input
Clang 14 263,400,261 -> 255,772,481 -> 241,915,419
GCC 12 266,098,949 -> 261,699,461 -> 247,682,262
Testing with -q -Ilib/ -D__GNUC__ lib/tokenize.cpp:
current -> std::istream input -> FILE* input
Clang 14 300,502,647 -> 294,632,802 -> 275,871,698
GCC 12 304,202,739 -> 301,878,425 -> 282,870,756
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
reducing the Ir count by using C I/O sounds good to me.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
…ntation-specific parts into StdIStream
… interface in reading initial file
…ome barebones documentation
|
I put this into Cppcheck with the current and added TokenList construction and all tests passed on Windows and Linux. So this is finally ready for review.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|