[Persist] Record diff metrics for TabPersistentStore observer events [chromium/src : main]
Calder Kitagawa (Gerrit)
Calder Kitagawa added 5 comments![]( "Open in Gerrit")
- Record the diff between time required to reach specific auth. andPlease use authoritative. I keep thinking authentication.
private void onStoreInitialized() {
Long authoritativeDuration = mAuthoritativeObserver.getInitializedDuration();
Long shadowDuration = mShadowObserver.getInitializedDuration();
if (authoritativeDuration == null || shadowDuration == null) return;
long diff = authoritativeDuration - shadowDuration;
if (diff > 0) {
RecordHistogram.recordTimesHistogram(
"Tabs.TabStateStore.LoadTimeDelta.Initialized.AuthoritativeLonger", diff);
} else if (diff < 0) {
RecordHistogram.recordTimesHistogram(
"Tabs.TabStateStore.LoadTimeDelta.Initialized.ShadowLonger", -diff);
}
}I suspect comparing these between arms will not be meaningful because the initialization order is different and one always happens first due to the shadow/authoritative always being created in a specific relative order. Rather than a diff I think we should record the timestamp this happens at relative to app startup for each store independently.
long diff = authoritativeDuration - shadowDuration;Given my comment below I think we should just independently record the durations of initialization for each i.e. the delta between start and load.
private void onStoreStateLoaded() {
Long authoritativeDuration = mAuthoritativeObserver.getStateLoadedDuration();
Long shadowDuration = mShadowObserver.getStateLoadedDuration();
if (authoritativeDuration == null || shadowDuration == null) return;
long diff = authoritativeDuration - shadowDuration;
if (diff > 0) {
RecordHistogram.recordTimesHistogram(
"Tabs.TabStateStore.LoadTimeDelta.StateLoaded.AuthoritativeLonger", diff);
} else if (diff < 0) {
RecordHistogram.recordTimesHistogram(
"Tabs.TabStateStore.LoadTimeDelta.StateLoaded.ShadowLonger", -diff);
}
mAuthoritativeStore.removeObserver(mAuthoritativeObserver);
mShadowStore.removeObserver(mShadowObserver);
}Comparing these things will not be apples to apples. One of the stores will create tabs and the other will not. The work to create tabs is the bulk of the time.
We can compare 1:1 between the same type of experiment arms (since authoritative will always create tabs), but it will never be valid to compare between the shadow and authoritative store.
[Android] Records the duration delta for the {Stage} event between the
authoritative and shadow stores when the respective store took longer.
Recorded when both stores have reached the {Stage} event.If you agree with my comments I'd recommend rewriting this description.
Related details
- Fiaz Muhammad
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Fiaz Muhammad (Gerrit)
Fiaz Muhammad voted and added 1 comment![]( "Open in Gerrit")
Votes added by Fiaz Muhammad
| Auto-Submit | +1 |
1 comment
- Record the diff between time required to reach specific auth. andPlease use authoritative. I keep thinking authentication.
- Calder Kitagawa
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Fiaz Muhammad (Gerrit)
Fiaz Muhammad voted and added 1 comment![]( "Open in Gerrit")
Votes added by Fiaz Muhammad
| Auto-Submit | +0 |
1 comment
private void onStoreStateLoaded() {
Long authoritativeDuration = mAuthoritativeObserver.getStateLoadedDuration();
Long shadowDuration = mShadowObserver.getStateLoadedDuration();
if (authoritativeDuration == null || shadowDuration == null) return;
long diff = authoritativeDuration - shadowDuration;
if (diff > 0) {
RecordHistogram.recordTimesHistogram(
"Tabs.TabStateStore.LoadTimeDelta.StateLoaded.AuthoritativeLonger", diff);
} else if (diff < 0) {
RecordHistogram.recordTimesHistogram(
"Tabs.TabStateStore.LoadTimeDelta.StateLoaded.ShadowLonger", -diff);
}
mAuthoritativeStore.removeObserver(mAuthoritativeObserver);
mShadowStore.removeObserver(mShadowObserver);
}Comparing these things will not be apples to apples. One of the stores will create tabs and the other will not. The work to create tabs is the bulk of the time.
We can compare 1:1 between the same type of experiment arms (since authoritative will always create tabs), but it will never be valid to compare between the shadow and authoritative store.
This is fair. I'm likely just going to abandon this CL, I agree that diffing them isn't a good metric.
I think it makes more sense to do add the metrics you mentioned in your earlier comment when we plan to make the TabStateStore authoritative. Then comparing the new implementation to the old one would actually be meaningful.
Fiaz Muhammad (Gerrit)
Fiaz Muhammad abandoned this change.
Fiaz Muhammad abandoned this change![]( "Open in Gerrit")
Related details
Code-Coverage
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |