Skip to content

Check if debug log is enabled before generate dashboard URL #3019

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

Merged
merged 1 commit into from
Oct 4, 2021

Conversation

jeremyjiang-dev
Copy link
Contributor

The PR will cut down the cost of generateDashboardURL and save ~10ms if debug log is disabled.

@google-cla google-cla bot added the cla: yes Override cla label Oct 1, 2021
@google-oss-bot
Copy link
Contributor

Coverage Report

Affected SDKs

No changes between base commit (1e58174) and head commit (2c610d9e).

Test Logs

Notes

HTML coverage reports can be produced locally with ./gradlew <product>:checkCoverage.
Report files are located at <product-build-dir>/reports/jacoco/.

Head commit (2c610d9e) is created by Prow via merging commits: 1e58174 02b2645.

@google-oss-bot
Copy link
Contributor

Binary Size Report

Affected SDKs

  • firebase-perf

    Type Base (1e58174) Head (2c610d9e) Diff
    aar 300 kB 300 kB +15 B (+0.0%)
    apk (aggressive) 980 kB 980 kB +80 B (+0.0%)
    apk (release) 2.45 MB 2.45 MB -40 B (-0.0%)

Test Logs

Notes

Head commit (2c610d9e) is created by Prow via merging commits: 1e58174 02b2645.

Copy link
Contributor

@visumickey visumickey left a comment

Choose a reason for hiding this comment

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

Wow! This is a steal. But something to think about is the cost of logging a message. Are we doing something wrong when we log messages? Worth pursuing with ACore/Android team to make sure they are aware of this.

@jeremyjiang-dev
Copy link
Contributor Author

/test check-changed

Copy link
Contributor

@leotianlizhan leotianlizhan left a comment

Choose a reason for hiding this comment

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

LGTM!

@jeremyjiang-dev jeremyjiang-dev merged commit 79975f0 into master Oct 4, 2021
@jeremyjiang-dev jeremyjiang-dev deleted the perfDashboardURL branch October 4, 2021 16:04
@firebase firebase locked and limited conversation to collaborators Nov 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants