Skip to content
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

Can't select rows via checkboxes anymore #884

Open
pyricau opened this issue Sep 10, 2024 · 11 comments
Open

Can't select rows via checkboxes anymore #884

pyricau opened this issue Sep 10, 2024 · 11 comments

Comments

@pyricau
Copy link

pyricau commented Sep 10, 2024

I'm not sure when this changed but up until recently I was able to select a time span and then checkboxes would appear on every row, and checking those checkboxes would add tabs to the bottom sheet that shows "Current selection". This is a great way to figure out where we spend time in various states, as well as how long each process is running across CPUs during that time span, etc.

As of today, it looks like the checkboxes are gone, I can't select rows anymore.

@LalitMaganti
Copy link
Collaborator

I cannot reproduce this:
image

The checkboxes show up fine and seem to do the right thing when I click on them (add/remove selections).

@pyricau
Copy link
Author

pyricau commented Sep 10, 2024

It.. just came back?!

@pyricau
Copy link
Author

pyricau commented Sep 10, 2024

oh, I know. If I manually select a span with click and slide the mouse, it works. But if I click on a span, press M, then click on the little triangles, the checkboxes don't show. They used to.

@LalitMaganti
Copy link
Collaborator

LalitMaganti commented Sep 10, 2024

Thanks for the clarification: I think then that change is somewhat intended. We were discussing internally that going from selection of an interval in the top bar -> area selection of tracks required a bunch of convoluted logic behind the scenes in the UI and was blocking improvements we want to make to how selection works.

We figured it was a very rarely used feature (after discussing with a bunch of internal users) and so we decided to (at least temporarily) stop coupling the two and unblock the other improvements. In fact in the multiple weeks this change has been in autopush/canary, you are the first person to notice this :)

@stevegolton is out this week but we can discuss when he is back if there's any way to bring it back easily in the post-refactoring world and/or some UX we can do to easily start an area selection from an interval selection.

@pyricau
Copy link
Author

pyricau commented Sep 10, 2024

It's definitely convoluted, and I accidentally stumbled on this, but it's a core part of my workflow now :) . I just didn't have anything to dig in lately.

The whole thing was definitely awkward, and even clicking the checkboxes on 8 CPU rows is very prone to error (if you mis click you lose the whole selection).

The thing I really want is being able to select an exact span rather than manually have to click and drag, which requires zooming in a lot and getting the mouse select exactly right.

The main two usages are:

  1. For a given trace section span, I want to visualize the sum of time spent in each cpu state
  2. For a given trace section span, I want to know how much CPU time is spent per process and per thread.

@LalitMaganti
Copy link
Collaborator

The whole thing was definitely awkward, and even clicking the checkboxes on 8 CPU rows is very prone to error (if you mis click you lose the whole selection).

So when I was saying "blocking improvements we want to make to how selection works", I was meaning exactly this :) We think that we can do a better job at giving people the information they are looking for without going through the awkward UX you have to go to today.

For a given trace section span, I want to visualize the sum of time spent in each cpu state

BTW there's already a better way to do this. If you click on a slice, you'll get the breakdown of the slice by cpu state:
image

For a given trace section span, I want to know how much CPU time is spent per process and per thread.

Yup similar to the above, we should have a pre-defined, opinionated aggregation for this and include it as another tab (named something like "other CPU consumers" or similar).

@stevegolton
Copy link
Member

We did indeed change this behaviour back in June (it probably only recently landed on the stable channel). I believe this was the commit.

The main thing that changed is area notes (what you create when you press M) no longer store a list of tracks, they just represent a start and end time. This change was made, as Lalit already mentioned, so that we could make some improvements to the arcitecture surrounding selection, and this feature was deemed to be fairly low priority so unfortunately it got removed.

What we still do have, and is a core feature of the UI, are are selections (this is what you create when clicking and dragging). Rather than making a span note, I think the feature you really need (correct me if I'm wrong) is a hotkey that allows you to turn the selected slice into an area selection. This would allow you to check the checkboxes.

Here is a demo (currently bound to 'L').

As you noted the selection management (click anywhere and lose the selection) leaves something to be desired in the UX department. Improving this is on our list.

@itstanany
Copy link

Here is a demo (currently bound to 'L').

Thanks @stevegolton for mentioning the new hotkey "L",
using it render the data correctly but a bug report pop up and only googlers have access to file the bug, so here is the report

UI: https://storage.googleapis.com/v47.0-2086bc583

Fully expanded statement
CREATE PERFETTO TABLE _flamegraph_filtered_16069318_83f9_481e_aa72_6277246f1f89 AS
^
Traceback (most recent call last):
File "stdin" line 1 col 1
CREATE PERFETTO TABLE _flamegraph_filtered_16069318_83f9_481e_aa72_6277246f1f89 AS
^
CREATE PERFETTO TABLE: SQLite error while creating body for table '_flamegraph_filtered_16069318_83f9_481e_aa72_6277246f1f89': Failed to bind pointer 21
Query:
CREATE PERFETTO TABLE _flamegraph_filtered_16069318_83f9_481e_aa72_6277246f1f89 AS
select *
from _viz_flamegraph_filter_frames!(
_flamegraph_source_16069318_83f9_481e_aa72_6277246f1f89,
0
)

  • (Error: Fully expanded statement)
  • WasmEngineProxy.query (/perfetto-ci-artifacts/20240916110241--cls-3267395-1--ui-clang-x86_64-release/uifrontend_bundle.js:197498:14)
  • async createPerfettoTable (/perfetto-ci-artifacts/20240916110241--cls-3267395-1--ui-clang-x86_64-release/uifrontend_bundle.js:69443:6)
  • async computeFlamegraphTree (/perfetto-ci-artifacts/20240916110241--cls-3267395-1--ui-clang-x86_64-release/uifrontend_bundle.js:180472:25)
  • async Object.work (/perfetto-ci-artifacts/20240916110241--cls-3267395-1--ui-clang-x86_64-release/uifrontend_bundle.js:180396:26)
  • async AsyncLimiter.runTaskQueue (/perfetto-ci-artifacts/20240916110241--cls-3267395-1--ui-clang-x86_64-release/uifrontend_bundle.js:67444:22)

Trace: not available (ARRAY_BUFFER). Provide repro steps.
UA: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/130.0.0.0 Safari/537.36
Referrer: https://github.com/

@LalitMaganti
Copy link
Collaborator

This is a bug which has been fixed on canary and autopush but is not yet released to stable. If you switch to one of them, things should work.

@itstanany
Copy link

This is a bug which has been fixed on canary and autopush but is not yet released to stable. If you switch to one of them, things should work.

thanks @LalitMaganti for fast reply, but in current canary release, the feature under discussion "turning selected slice into an area selection" either by the hotkey "M" or "L" as in the new in this version is not working at all

@LalitMaganti
Copy link
Collaborator

LalitMaganti commented Nov 6, 2024

We landed the change [1] Steve was talking about in #884 (comment) but there was some bikesheddding about the exactly which hotkey we want to bind to so we landed it without the hotkey.

It's accessible in the Command Palette: if you have selected something you can do Ctrl + Shift + P -> "Convert the current selection to an area selection" to get the same behaviour as the L hotkey in the demo Steve linked.

[1] https://cs.android.com/android/platform/superproject/main/+/main:external/perfetto/ui/src/frontend/ui_main.ts;l=359?q=%22%20area%20selection%22%20f:perfetto

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

No branches or pull requests

4 participants