Skip to content

Use AQS for Firebase Performance sessions. #6665

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

Closed
wants to merge 150 commits into from
Closed

Conversation

tejasd
Copy link
Contributor

@tejasd tejasd commented Feb 3, 2025

No description provided.

Copy link
Contributor

github-actions bot commented Feb 3, 2025

Release note changes

No release note changes were detected. If you made changes that should be
present in the next release, ensure you've added an entry in the appropriate
CHANGELOG.md file(s).

Copy link
Contributor

github-actions bot commented Feb 3, 2025

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: v6.1

@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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Feb 3, 2025

Coverage Report 1

Affected Products

No changes between base commit (fcd270c) and merge commit (eed2dc5).

Test Logs

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

Copy link
Contributor

github-actions bot commented Feb 3, 2025

Test Results

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

Results for commit 8738761.

♻️ This comment has been updated with latest results.

@tejasd tejasd changed the base branch from main to fireperf-aqs February 5, 2025 18:22
VinayGuthal and others added 17 commits April 1, 2025 12:09
Bidirectional streaming for android. Creates a bunch of helper classes
for the same. The main classes which handle the bidirectional streaming
are LiveGenerativeModel and LiveSession

---------

Co-authored-by: VinayGuthal <[email protected]>
Co-authored-by: Rodrigo Lazo Paz <[email protected]>
Co-authored-by: Rodrigo Lazo <[email protected]>
Co-authored-by: Daymon <[email protected]>
…-changelog (#6809)

This change adds a way to repeatedly log identical performance traces in
different processes.
- Currently traces on different activities are logged on different
Fireperf Sessions.
- Adds additional logging to help identify different Firebase instances
in different processes.
- Adds Fireperf custom attributes to identify processes.
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.
This PR doesn't change the use of session ID to AQS - except in
GaugeMetadata. I've added TODOs to identify the missing locations.
Use multi-process DataStore instead of Preferences DataStore.

This change allows multiple processes to share the same datastore file
safely. This reduces settings fetch to one per app run, not one per
process.

Also updated the TimeProvider to provide an object with explicit time
units. This will make time less error prone. Removed all instances of
`System.currentTimeMillis()` from tests, making them deterministic.
With the multi-process data store change, all processes will read the
settings cache from the same file safely. This means if a second process
started, it would read the cache the first process persisted.

But if 2 processes were already running, and one fetched and cached new
settings, it wouldn't automatically share it with the other running
process.

This change fixes that by having all processes watch the file.

Also cleaned up settings a bit, and made everything in seconds to avoid
converting units. Cleaned up unit tests. Also removed the need to lazy
load the cache by doing a double check in the getter instead.

There is more potential to clean up, but let's do it later.
@mrober mrober force-pushed the fireperf-aqs branch 2 times, most recently from 60f2e77 to 7d9c0bf Compare April 7, 2025 17:26
@tejasd tejasd closed this May 5, 2025
@tejasd tejasd deleted the td/fireperf-aqs branch May 5, 2025 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.