Skip to content

Allow profiling queries #91

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 10 commits into from
May 22, 2025
Merged

Allow profiling queries #91

merged 10 commits into from
May 22, 2025

Conversation

simolus3
Copy link
Contributor

This adds the optional profileQueries option to SqliteOptions. When enabling it, all queries and arguments will be logged to the timeline with dart:developer APIs (that delegate to globalSelf.performance on web platforms).

  • In workers, we use the synchronous timeline APIs. Unfortunately, this doesn't seem to have much of an effect because Chrome doesn't allow attaching a profiler to shared workers.
  • We use the async TimelineTask APIs to measure query times outside of workers. On the web, this creates two events (xxx-begin and xxx-end and then calls performance.measure to create a mark betwen them).

@simolus3 simolus3 requested a review from rkistner May 19, 2025 15:17
@simolus3
Copy link
Contributor Author

pana is failing due to the pending sqlite_async update

Copy link
Contributor

@rkistner rkistner left a comment

Choose a reason for hiding this comment

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

Thanks, I think this will be a great tool for debugging.

I've experimented a bit with this, and some comments so far:

Web

  1. getAll / execute appear like very generic terms in the timeline.
  2. I couldn't find the arguments passed in on the events displayed anywhere.

Could we instead using something like SQL: $sql as the event name? That would make it easy to see what is happening on the timeline:
image

  1. _ExclusiveContext.execute / executeBatch is currently missing on the timeline.

Native

I've found it quite difficult to find my way around the Dart/Flutter debugger. Eventually settled on this workflow:

  1. Open the debugger, Performance tab, Timeline Events.
  2. Click "Clear All".
  3. Perform the actions to time in the app.
  4. Click "Refresh Timeline Events".

Now you can find the timeline events. If the events are all short, you may still struggle.

Here it seems like we have two options for how the events are displayed:

  1. TimelineTask, like you're using currently. There, the name is used for both the track and the task name. That means it's we can't use the query in the task name - that can end up with events on hundreds of separate tracks.
  2. Use Timeline.startSync instead, with the sql queries in the task name. Then the events all end up on DartWorker tracks, with the individual names. But with our connection pooling, these will end up on arbitrary worker tracks, so still difficult to find.

This is also mentioned here: dart-lang/sdk#56274

I think given those two options, the TimelineTask like you currently have is probably best. I think we can just consider more explicit names for the tasks, with a common prefix, e.g. sqlite:getAll, sqlite:execute, etc. The common prefix will let all the tracks appear next to each other.

Another alternative to consider is just logging queries to the console, perhaps in addition to the timeline. Not sure if we should make the two configurable individually.

General comments

We can consider making the timeline events enabled by default in debug builds - I think that's also the case for the built-in Flutter timeline events, and should not add more overhead than those.

@simolus3
Copy link
Contributor Author

I couldn't find the arguments passed in on the events displayed anywhere.

There're attached to the individual events making up the slice (under event log -> search for sqlite_async):

Details view

Could we instead using something like SQL: $sql as the event name? That would make it easy to see what is happening on the timeline:

I've added the sqlite_async prefix for easier searching on all platforms, on web I'm also including the SQL in the event name now.

_ExclusiveContext.execute / executeBatch is currently missing on the timeline.

Thanks, fixed 👍

I think given those two options, the TimelineTask like you currently have is probably best.

A problem is also that Timeline.startSync must be finished before yielding to the event loop. So we could use it when tracking this on the background isolate, but then there's no way to associate Timeline.startSync with the parent TimelineTask from the calling isolates which is unfortunate.

We can consider making the timeline events enabled by default in debug builds - I think that's also the case for the built-in Flutter timeline events, and should not add more overhead than those.

I agree, but we should probably also enable this by default for profile builds.

@rkistner
Copy link
Contributor

Ah, I wasn't aware of that search in the event log. I could swear I sometimes saw the events flashing while it was loading, before being replaced by a function call tree. But searching brings those up again.

Either way, it's good to know that is available for getting the args passed in now, but can be avoided in most cases where the query itself is sufficient.

rkistner
rkistner previously approved these changes May 20, 2025
Browsers don't surface this information either way
rkistner
rkistner previously approved these changes May 20, 2025
@simolus3
Copy link
Contributor Author

I've removed changes to prepare a release for this branch - I want took into an issue cancelling stream subscriptions too before preparing the release.

@simolus3 simolus3 merged commit bb65cfa into main May 22, 2025
6 checks passed
@simolus3 simolus3 deleted the timeline branch May 22, 2025 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants