-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
Fix performance regression introduced with the parsing callback feature. #69
Conversation
Fix performance regression introduced with the parsing callback feature.
Thanks!!! |
Hi @aburgh, I have another question... Sorry for bothering you again. Do you have an idea how to cover theses lines with test cases:
All the best |
Hi @aburgh, I have another issue related with performance: When an object is parsed and the key is processed, the following code is executed: // store key
expect(lexer::token_type::value_string);
const string_t key = m_lexer.get_string();
bool keep_tag = false;
if (keep)
{
keep_tag = callback(depth, parse_event_t::key, basic_json(key));
} Thereby, |
Did you add test cases? I stepped through the unit tests and I see the tests in this section do provide code coverage:
|
They do. But I fail to cover the two lines I mentioned above. |
I'm confused about what Coverall reports as coverage. Those two lines of code are executed by the test that I referred to, I can see that in the debugger. Is executing the code not the definition of coverage? |
Hi Niels,
Upon reviewing the changes I made during our exchange regarding the performance of the callback feature, these two moves appear to account for all of the improvement to get performance back to the non-callback version.
Regards,
Aaron