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

Do not create trace log message when trace logging is not enabled #544

Merged
merged 6 commits into from
May 8, 2023

Conversation

lennehendrickx
Copy link
Contributor

The "Can't take as work ..." trace message is created even when trace logging is not enabled. In our profiler we saw that this generates a significant amount of garbage. This PR only generates the message if trace logging is enabled.

Checklist

  • Documentation (if applicable)
  • Changelog

@lennehendrickx
Copy link
Contributor Author

Hi Confluent Team,

This is generating quite some unnecessary garbage in our applications. Did you already have time to look into this? Or is there something we can/should do before this PR is considered?

Thanks!

@what-the-diff
Copy link

what-the-diff bot commented Mar 21, 2023

PR Summary

  • Optimized log tracing - Improved performance by checking if trace logging is enabled before executing trace statements
  • New method in ProcessingShard class - Added cantTakeAsWorkMsg(...) method to aid in handling exceptional cases when assigning work messages

@lennehendrickx
Copy link
Contributor Author

Hi @rkolesnev,

I have added some more conditional checks to avoid unnecessary calculation of properties for trace logging when trace logging is not active.

@what-the-diff
Copy link

what-the-diff bot commented Apr 18, 2023

PR Summary

  • Removed unnecessary log statements
    Unnecessary log.trace() and log.debug() statements were removed, making the code cleaner.

  • Added cantTakeAsWorkMsg(...) method to ProcessingShard
    A new method called cantTakeAsWorkMsg(...) was added, providing better control over logging information during function calls, improving performance, and reducing unnecessary logs.

  • Optimized logging in WorkManager
    The logging in WorkManager has been optimized, only printing messages if debug logging is enabled, reducing resource usage and improving overall application performance.

@lennehendrickx
Copy link
Contributor Author

Hi @rkolesnev ,

Did the team already have time to look into this?

Thanks

@rkolesnev rkolesnev merged commit 632f9da into confluentinc:master May 8, 2023
@rkolesnev
Copy link
Contributor

Hi @rkolesnev ,

Did the team already have time to look into this?

Thanks

Approved, merged.

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.

2 participants