Skip to content
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

IsLoggingEnabled() boilerplate could be removed #193

Open
dechamps opened this issue Jun 20, 2023 · 0 comments
Open

IsLoggingEnabled() boilerplate could be removed #193

dechamps opened this issue Jun 20, 2023 · 0 comments

Comments

@dechamps
Copy link
Owner

Most places where IsLoggingEnabled() is used don't actually benefit from any performance benefit from conditional logging (e.g. they are logging a string literal which is trivially cheap anyway).

It could be that these are remnants from a previous implementation of Log() that fed the operands into a std::stringstream unconditionally. That is not the case today - operands are not formatted if logging is disabled.

We should still be careful to not evaluate operands that are expensive at the Log() call site, but even in this case, there might be cleaner ways of achieving the same outcome. For example, DescribeASIOTime() which is used as operand in some places could be improved to take the form of a class with an overloaded operator<<(), instead of returning a string, such that no actual string construction takes place until actual streaming.

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

No branches or pull requests

1 participant