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

Add supplier pattern to avoid message creation overhead. #100

Closed
wants to merge 1 commit into from
Closed

Add supplier pattern to avoid message creation overhead. #100

wants to merge 1 commit into from

Conversation

thespags
Copy link

The supplier will only be invoked if the log line's level is met. Otherwise we avoid string creation. This is to optimize applications that may want to minimize extra string creation overhead.

@@ -13,6 +13,10 @@ var _ Interface = (*Entry)(nil)
// Now returns the current time.
var Now = time.Now

// logSupplier Allows lazy creation of strings for logging.
// If the log line is skipped then this supplier will not be invoked.
type logSupplier func() string
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The supplier function that will be invoked.

if level < l.Level {
return
}
l.log(level, e, msg())
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skips invoking msg() if level is not met otherwise creates the message and defer to the main logging flow.

Level: log.InfoLevel,
}

l.Debugl(func() string { assert.Fail(t, "Supplier should not be invoked."); return "ignored"})
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests that this supplier is never invoked otherwise the test would fail. If I remove line 181 with the level skip the test fails.

@thespags
Copy link
Author

Hi @tj. I was wondering how often you get a chance to review PRs to this repo. I noticed there wasn't any updates in the last year. Thank you!

@thespags thespags closed this by deleting the head repository Jul 13, 2023
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.

None yet

1 participant