Skip to content

Update GaugeManager and SessionManager to separate app state changes and PerfSession changes #6876

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 10 commits into from
Closed

Update GaugeManager and SessionManager to separate app state changes and PerfSession changes #6876

wants to merge 10 commits into from

Conversation

tejasd
Copy link
Contributor

@tejasd tejasd commented Apr 15, 2025

No description provided.

tejasd and others added 5 commits April 7, 2025 13:17
@tejasd @mrober
Remove the logging of GaugeMetadata to allow using AQS (#6678)
10369d0 
Based on the behaviour of AQS w/ Fireperf, an AQS session isn't
available when (currently) logging gauge metadata.

Changes:
- Remove the current logging of gauge metadata - will be re-introduced
in a future PR.
- Switch Gauge collection from `scheduleAtFixedRate` to
`scheduleWithFixedDelay`. As
[documented](https://stackoverflow.com/a/78405653), this *should*
prevent a potentially large amounts of gauge collection if a process is
cached, and then restored during a verbose session - which *should* make
it work better w/ AQS.
- Remove API restricted behaviour which is no longer relevant.
@tejasd @mrober
Implement a SessionSubscriber for Firebase Performance (#6683)
a9f47b4 
This PR doesn't change the use of session ID to AQS - except in
GaugeMetadata. I've added TODOs to identify the missing locations.
@themiswang @mrober
convert perf session to use aqs support session id (#6832)
7d9c0bf 
- Override PerfSession session id to aqs session id when possible
- Add assertion for debug mode to double check if aqs session id
available by the time of use
@tejasd
Update PerfSession and SessionManager to identify Legacy sessions. (#…
8fe1383 
…6867)

This is a subset of changes in
[#6693](#6693).
@tejasd
init
0d9ba90
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 15, 2025 •
edited
Loading

📝 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.

@github-actions GitHub Actions
Copy link
Contributor

Vertex AI Mock Responses Check ⚠️

A newer major version of the mock responses for Vertex AI unit tests is available. update_responses.sh should be updated to clone the latest version of the responses: v10.0

@tejasd tejasd changed the base branch from main to fireperf-aqs April 15, 2025 16:07
@google-oss-bot
Copy link
Contributor

1 Warning
⚠️ Did you forget to add a changelog entry? (Add the 'no-changelog' label to the PR to silence this warning.)

Generated by 🚫 Danger

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 15, 2025 •
edited
Loading

Test Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit 3120db5.

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Apr 15, 2025 •
edited
Loading

Coverage Report 1

Affected Products

No changes between base commit (009c1c1) and merge commit (659bf32).

Test Logs

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Apr 15, 2025 •
edited
Loading

Size Report 1

Affected Products

  • base

    TypeBase (8fe1383)Merge (efa78d8)Diff
    apk (aggressive)?8.80 kB? (?)
    apk (release)?9.77 kB? (?)

  • firebase-perf

    TypeBase (8fe1383)Merge (efa78d8)Diff
    aar?321 kB? (?)
    apk (aggressive)?1.71 MB? (?)
    apk (release)?10.7 MB? (?)

  • firebase-sessions

    TypeBase (8fe1383)Merge (efa78d8)Diff
    aar?202 kB? (?)
    apk (aggressive)?645 kB? (?)
    apk (release)?6.31 MB? (?)

  • protolite-well-known-types

    TypeBase (8fe1383)Merge (efa78d8)Diff
    aar?1.02 MB? (?)
    apk (aggressive)?134 kB? (?)
    apk (release)?1.71 MB? (?)

Test Logs

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

@google-oss-bot
Copy link
Contributor

Startup Time Report 1

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

Startup time comparison between the CI merge commit (acbffec) and the base commit (5aff679) are not available.

No macrobenchmark data found for the base commit (5aff679). Analysis for the CI merge commit (acbffec) can be found at:

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

@tejasd tejasd closed this May 5, 2025
@tejasd tejasd deleted the td/gauge-manager branch May 5, 2025 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tejasd @google-oss-bot @themiswang