Repository navigation
Build the log messages of string lookups only when logging is on - #30
Merged
Merged
Conversation
ResourcesUtil and Repository passed finished strings to the logger, which
only then checked the level. At LingoHubLogLevel.NONE, the default in
release builds, every getString() still built about six messages that
were thrown away, and looked up the resource type name for one of them.
These lookups run for every getString() and stringResource() in
Activities, often during view inflation and Compose recomposition, and
with LingoHub.wrap() (org #2350) for the application context and
Services as well.
LingoHubLogger.debug {} and warn {} check the level first and build the
message only at FULL. The messages at FULL stay the same.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
At
LingoHubLogLevel.NONE(the default in release builds), string lookups still built their debug messages, because the level was only checked inside the logger. That meant about six throwaway strings pergetString(), plus agetResourceTypeName()call that only fed one of them. The lookup path runs during view inflation and Compose recomposition, and withLingoHub.wrap()(#29) also for the application context and Services.LingoHubLoggergets internal inlinedebug {}/warn {}that check the level before building the message.ILingoHubLoggeris unchanged, and messages atFULLstay the same.ResourcesUtilandRepositorylog through them.LingoHubContextWrapper.getResources()is left alone: its per-call message is a compile-time constant, and #2350 Serve downloaded translations to Services, notifications and the application context with LingoHub.wrap() #29 rewrites that file.Verified:
./gradlew buildpasses (98 unit tests including the newLingoHubLoggerTest, lint), andjavapshows every log message inResourcesUtilandRepositorybuilt only behind the level check. The branch merges cleanly with #29, and the merged tree builds too.