Repository navigation
Fix Performance page loading forever when the systrace recorder is in use - #10054
ishaquehassan wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request prevents the Performance page from loading indefinitely when the connected app uses a timeline recorder (such as systrace) that does not store events in the Dart VM. It handles the kInvalidTimelineRequest RPC error, stops polling attempts, and displays a warning notification to the user. The feedback suggests optimizing resource usage by canceling the background polling timer and disposing of the rate limiter once the timeline is determined to be unavailable. Additionally, the release notes entry should be updated to link to the pull request instead of the issue, in accordance with the repository's style guide.
| _vmTimelineUnavailable = true; | ||
| _log.info(e.details ?? e.message); | ||
| pushVmTimelineUnavailableWarning(); |
There was a problem hiding this comment.
[CONCERN] When the VM timeline is determined to be unavailable, we should cancel the periodic polling timer and dispose of the rate limiter. This avoids leaving a useless periodic timer running every 10 seconds in the background.
_vmTimelineUnavailable = true;
_pollingTimer?.cancel();
_pollingTimer = null;
_timelinePollingRateLimiter?.dispose();
_timelinePollingRateLimiter = null;
_log.info(e.details ?? e.message);
pushVmTimelineUnavailableWarning();| * Fixed the Performance page loading forever when the connected app uses a | ||
| timeline recorder that DevTools cannot read from, such as systrace. | ||
| [#6524](https://github.com/flutter/devtools/issues/6524) |
There was a problem hiding this comment.
[NIT] According to the DevTools release notes guidelines, the link following the entry should be the PR link (e.g., [#PR_NUMBER](https://github.com/flutter/devtools/pull/PR_NUMBER)) rather than the issue link. Please update this with the PR number once it is available.
This branch has not been deployed
2 participants
Fixes #6524
Fixes #9927
When the app uses a timeline recorder that doesn't keep events in the VM (systrace via
--trace-systrace, or Android with ATrace already on at startup like in #9927), the VM answersgetPerfettoVMTimelinewith RPC error 114 (kInvalidTimelineRequest). Nothing caught it, soTimelineEventsController.initthrew,PerformanceController._initHelperbailed out of its.waitbefore subscribing toFlutter.Frameevents, andinitializednever completed. That's the endless spinner on the Performance page.Following @bkonyi's suggestion in #6524, this gates on that response:
_forceRefreshand the polling rate limiter now go through_maybePullPerfettoVmTimeline, which catches onlyRPCErrors withRPCErrorKind.kInvalidTimelineRequest.codeand rethrows anything else. On that error it remembers that the VM timeline is unavailable, logs the VM's reason, and shows a dismissible notification saying timeline events aren't available because the app uses a recorder like systrace, and that restarting without system tracing brings them back._initForServiceConnectiondoesn't start the polling timer in that case, and the "events occurred too long ago" warning on frame selection is skipped, since that advice doesn't apply.With
initno longer throwing, the controller finishes initializing and listens for frame and rebuild events as usual, so only the Timeline Events tab stays empty. I checked the RPCs against a real VM started with--timeline_recorder=systrace: bothgetPerfettoVMTimelineand the DDSgetPerfettoVMTimelineWithCpuSamplescall (used when "Include CPU samples" is on) return code 114, whilesetVMTimelineFlags,getVMTimelineMicrosandclearVMTimelinestill succeed, so nothing else in init needed changes. I haven't run the DevTools UI against a systrace app, so there are no screenshots.The new test in
timeline_events_controller_test.dartmocks the service to throw that error and checks thatinit()completes, that a second refresh doesn't hit the VM again, and that the notification is shown. It fails on master with the samegetPerfettoVMTimeline: (114)error from the issue.timeline_events_controller_test.dartpasses 4/4 andtest/screens/performance/passes 111/111.Pre-launch Checklist
General checklist
///).Issues checklist
contributions-welcomeorgood-first-issuelabel.contributions-welcomeorgood-first-issuelabel. I understand this means my PR might take longer to be reviewed.Tests checklist
AI-tooling checklist
Feature-change checklist
release-notes-not-requiredlabel or left a comment requesting the label be added.packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md.If you need help, consider asking for help on Discord.