Skip to content

Check if the trace times are not NULL before creating the AppStartTrace. #4762

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 6 commits into from
May 18, 2023

Conversation

visumickey
Copy link
Contributor

Check if the trace times are not NULL before creating the AppStartTrace. Fixes #4730

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2023

📝 PRs merging into main branch

Our main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released.

Add the 'main-merge-ack' label to your PR to confirm merging into the main branch is intended.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Mar 9, 2023

Coverage Report 1

Affected Products

  • firebase-perf

    Overall coverage changed from 71.18% (aa2a424) to 71.19% (65af9ce) by +0.00%.

    FilenameBase (aa2a424)Merge (65af9ce)Diff
    AppStartTrace.java77.27%77.37%+0.09%

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/TgJ1nz3KKW.html

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2023

Unit Test Results

   102 files   -    692     102 suites   - 692   3m 12s ⏱️ - 33m 30s
   975 tests  - 3 934     975 ✔️  - 3 913  0 💤  - 21  0 ±0 
1 950 runs   - 7 777  1 950 ✔️  - 7 735  0 💤  - 42  0 ±0 

Results for commit ca2564e. ± Comparison against base commit aa2a424.

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Mar 9, 2023

Size Report 1

Affected Products

  • firebase-perf

    TypeBase (aa2a424)Merge (65af9ce)Diff
    aar315 kB315 kB+42 B (+0.0%)
    apk (aggressive)957 kB957 kB-28 B (-0.0%)
    apk (release)2.95 MB2.95 MB-28 B (-0.0%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/pdRzxqfdrO.html

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Mar 9, 2023

Startup Time Report 1

Note: Layout is sometimes suboptimal due to limited formatting support on GitHub. Please check this report on GCS.

Notes

Startup Times

  • fire-perf

    DeviceStatisticsDistributions
    oriole-32
    Percentileaa2a42465af9ceDiffSignificant (?)
    p10275 ±21 μs240 ±19 μs-35.1 μs (-12.8%)NO
    p25289 ±22 μs250 ±18 μs-38.1 μs (-13.2%)NO
    p50306 ±24 μs269 ±17 μs-36.9 μs (-12.0%)NO
    p75338 ±23 μs301 ±17 μs-37.0 μs (-10.9%)NO
    p90392 ±33 μs363 ±18 μs-28.7 μs (-7.3%)NO

    20 test runs in comparison
    CommitTest Runs
    aa2a424
    • 2023-05-18_04:41:00.366168_WvHy
    • 2023-05-18_04:41:00.369291_CAXw
    • 2023-05-18_04:41:00.369314_rRTb
    • 2023-05-18_04:41:00.369323_kBrp
    • 2023-05-18_04:41:00.369330_SCFK
    • 2023-05-18_04:41:00.369345_bDNb
    • 2023-05-18_04:41:00.369353_Pqzu
    • 2023-05-18_04:41:00.369361_KZkT
    • 2023-05-18_04:41:00.369368_FHEb
    • 2023-05-18_04:41:00.369375_bTBM
    65af9ce
    • 2023-05-18_21:13:59.073778_WOsz
    • 2023-05-18_21:13:59.076396_nkTV
    • 2023-05-18_21:13:59.076410_InBJ
    • 2023-05-18_21:13:59.076416_URPw
    • 2023-05-18_21:13:59.076422_eAhL
    • 2023-05-18_21:13:59.076428_rbZJ
    • 2023-05-18_21:13:59.076434_jyNK
    • 2023-05-18_21:13:59.076440_esey
    • 2023-05-18_21:13:59.076445_vEwF
    • 2023-05-18_21:13:59.076451_MtYf
    redfin-30
    Percentileaa2a42465af9ceDiffSignificant (?)
    p10817 ±79 μs776 ±79 μs-40.8 μs (-5.0%)NO
    p25852 ±66 μs817 ±63 μs-34.9 μs (-4.1%)NO
    p50913 ±56 μs874 ±57 μs-39.2 μs (-4.3%)NO
    p751.00 ±0.08 ms947 ±76 μs-54.9 μs (-5.5%)NO
    p901.16 ±0.1 ms1.08 ±0.07 ms-73.3 μs (-6.3%)NO

    20 test runs in comparison
    CommitTest Runs
    aa2a424
    • 2023-05-18_04:41:00.366168_WvHy
    • 2023-05-18_04:41:00.369291_CAXw
    • 2023-05-18_04:41:00.369314_rRTb
    • 2023-05-18_04:41:00.369323_kBrp
    • 2023-05-18_04:41:00.369330_SCFK
    • 2023-05-18_04:41:00.369345_bDNb
    • 2023-05-18_04:41:00.369353_Pqzu
    • 2023-05-18_04:41:00.369361_KZkT
    • 2023-05-18_04:41:00.369368_FHEb
    • 2023-05-18_04:41:00.369375_bTBM
    65af9ce
    • 2023-05-18_21:13:59.073778_WOsz
    • 2023-05-18_21:13:59.076396_nkTV
    • 2023-05-18_21:13:59.076410_InBJ
    • 2023-05-18_21:13:59.076416_URPw
    • 2023-05-18_21:13:59.076422_eAhL
    • 2023-05-18_21:13:59.076428_rbZJ
    • 2023-05-18_21:13:59.076434_jyNK
    • 2023-05-18_21:13:59.076440_esey
    • 2023-05-18_21:13:59.076445_vEwF
    • 2023-05-18_21:13:59.076451_MtYf
  • timeToInitialDisplay

    DeviceStatisticsDistributions
    oriole-32
    Percentileaa2a42465af9ceDiffSignificant (?)
    p10194 ±8 ms195 ±4 ms+1.76 ms (+0.9%)NO
    p25200 ±8 ms200 ±4 ms+613 μs (+0.3%)NO
    p50207 ±8 ms208 ±4 ms+1.20 ms (+0.6%)NO
    p75216 ±9 ms217 ±4 ms+1.58 ms (+0.7%)NO
    p90225 ±12 ms229 ±7 ms+4.12 ms (+1.8%)NO

    20 test runs in comparison
    CommitTest Runs
    aa2a424
    • 2023-05-18_04:41:00.366168_WvHy
    • 2023-05-18_04:41:00.369291_CAXw
    • 2023-05-18_04:41:00.369314_rRTb
    • 2023-05-18_04:41:00.369323_kBrp
    • 2023-05-18_04:41:00.369330_SCFK
    • 2023-05-18_04:41:00.369345_bDNb
    • 2023-05-18_04:41:00.369353_Pqzu
    • 2023-05-18_04:41:00.369361_KZkT
    • 2023-05-18_04:41:00.369368_FHEb
    • 2023-05-18_04:41:00.369375_bTBM
    65af9ce
    • 2023-05-18_21:13:59.073778_WOsz
    • 2023-05-18_21:13:59.076396_nkTV
    • 2023-05-18_21:13:59.076410_InBJ
    • 2023-05-18_21:13:59.076416_URPw
    • 2023-05-18_21:13:59.076422_eAhL
    • 2023-05-18_21:13:59.076428_rbZJ
    • 2023-05-18_21:13:59.076434_jyNK
    • 2023-05-18_21:13:59.076440_esey
    • 2023-05-18_21:13:59.076445_vEwF
    • 2023-05-18_21:13:59.076451_MtYf
    redfin-30
    Percentileaa2a42465af9ceDiffSignificant (?)
    p10233 ±5 ms256 ±4 ms+22.7 ms (+9.7%)YES
    p25239 ±4 ms262 ±4 ms+22.7 ms (+9.5%)YES
    p50247 ±5 ms270 ±5 ms+22.1 ms (+8.9%)MAYBE
    p75255 ±6 ms280 ±6 ms+24.3 ms (+9.5%)MAYBE
    p90264 ±6 ms292 ±9 ms+28.1 ms (+10.6%)MAYBE

    20 test runs in comparison
    CommitTest Runs
    aa2a424
    • 2023-05-18_04:41:00.366168_WvHy
    • 2023-05-18_04:41:00.369291_CAXw
    • 2023-05-18_04:41:00.369314_rRTb
    • 2023-05-18_04:41:00.369323_kBrp
    • 2023-05-18_04:41:00.369330_SCFK
    • 2023-05-18_04:41:00.369345_bDNb
    • 2023-05-18_04:41:00.369353_Pqzu
    • 2023-05-18_04:41:00.369361_KZkT
    • 2023-05-18_04:41:00.369368_FHEb
    • 2023-05-18_04:41:00.369375_bTBM
    65af9ce
    • 2023-05-18_21:13:59.073778_WOsz
    • 2023-05-18_21:13:59.076396_nkTV
    • 2023-05-18_21:13:59.076410_InBJ
    • 2023-05-18_21:13:59.076416_URPw
    • 2023-05-18_21:13:59.076422_eAhL
    • 2023-05-18_21:13:59.076428_rbZJ
    • 2023-05-18_21:13:59.076434_jyNK
    • 2023-05-18_21:13:59.076440_esey
    • 2023-05-18_21:13:59.076445_vEwF
    • 2023-05-18_21:13:59.076451_MtYf

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/jnELCEi8Uo/index.html

Copy link
Member

@raymondlam raymondlam left a comment

Choose a reason for hiding this comment

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

It would be nice if there were some tests for these edge cases. I'm not sure how easy it is for that.
Seeing that there is some trickiness to these cases, I would recommend removing the subtraces rather than keeping them.

@visumickey
Copy link
Contributor Author

It would be nice if there were some tests for these edge cases. I'm not sure how easy it is for that. Seeing that there is some trickiness to these cases, I would recommend removing the subtraces rather than keeping them.

I thought about this. But felt the changes would be a lot more than the scope of this PR. So, will create a separate PR to remove all these experiments.

@visumickey visumickey merged commit 28e3a68 into master May 18, 2023
@visumickey visumickey deleted the perf4730 branch May 18, 2023 21:36
@firebase firebase locked and limited conversation to collaborators Jun 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

com.google.firebase.perf.util.Timer.getDurationMicros NullPointerException
4 participants