Skip to content

astutils.cpp: fixed suboptimal string comparisons / small cleanup#6799

Merged
firewave merged 3 commits intocppcheck-opensource:mainfrom
firewave:astflatten
Sep 26, 2024
Merged

astutils.cpp: fixed suboptimal string comparisons / small cleanup#6799
firewave merged 3 commits intocppcheck-opensource:mainfrom
firewave:astflatten

Conversation

@firewave
Copy link
Copy Markdown
Collaborator

No description provided.

@firewave firewave changed the title astutils.cpp: fixed suboptimal string comparisons astutils.cpp: fixed suboptimal string comparisons / small cleanup Sep 14, 2024
@chrchr-github
Copy link
Copy Markdown
Collaborator

What is the performance gain here? Why would strcmp be faster than ==?

@firewave
Copy link
Copy Markdown
Collaborator Author

What is the performance gain here?

Less than a percent in the example I tested but as usual that depends how hot the path is.

Why would strcmp be faster than ==?

It gets rid of the strlen() to determine the length of of the const char* to compare with.

@chrchr-github
Copy link
Copy Markdown
Collaborator

chrchr-github commented Sep 16, 2024

Why would strcmp be faster than ==?

It gets rid of the strlen() to determine the length of of the const char* to compare with.

Seems like all three compilers do strlen + memcmp in operator== as well as std::string::compare(). Maybe it depends on the use case which one is faster? We probably only check very short strings.

@firewave
Copy link
Copy Markdown
Collaborator Author

We probably only check very short strings.

I think it is always a single character. I would have just changed it to char but there is one instance where a pointer is passed through. Maybe that can be fixated.

@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Sep 26, 2024

when I benchmarked in quick bench I did not see a difference in performance but perhaps my benchmark was not well written.

@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Sep 26, 2024

when I benchmarked in quick bench I did not see a difference in performance but perhaps my benchmark was not well written.

in benchmark I wrote and executed locally it seems the strcmp is considerably faster.

int direct(const std::string& s, const char* op) {
    return s == op;
}

int use_strcmp(const std::string& s, const char* op) {
    return std::strcmp(s.c_str(), op);
}

👍

@firewave
Copy link
Copy Markdown
Collaborator Author

There's also benchmark in these tickets:
https://trac.cppcheck.net/ticket/11253
https://trac.cppcheck.net/ticket/11252

@firewave firewave merged commit b14f100 into cppcheck-opensource:main Sep 26, 2024
@firewave firewave deleted the astflatten branch September 26, 2024 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants