awslabs / damo

DAMON user-space tool
https://damonitor.github.io/
GNU General Public License v2.0
148 stars 28 forks source link

damo show enhancement #69

Closed honggyukim closed 11 months ago

honggyukim commented 11 months ago

Hi SeongJae,

It'd be useful if damo show supports the followings.

  1. print the total number of regions at the top I think running damo show with watch command is useful, but it shows only the top part of damo show result and keep updating it. If there are about hundreds of regions then there is no way to know how many regions are created. For example, the output can be printed as follows.

    $ ./damo show
    total number of regions: 13
    0 addr [4.000 KiB, 640.000 KiB) (636.000 KiB) access 0 % age 100 ms
    1 addr [1024.000 KiB, 3.000 GiB) (2.999 GiB) access 0 % age 100 ms
    2 addr [4.000 GiB, 19.579 GiB) (15.579 GiB) access 0 % age 200 ms
        ...
    total sz: 160.000 GiB
  2. provide a way to sort the regions (based on access rate or age) The current damo show output prints the regions in order based on the address. This can be useful, but users can also be interested in most accessed regions or oldest regions.

I would like to hear how you think about it. Thanks.

sj-aws commented 11 months ago

Hi Honggyu,

print the total number of regions at the top

Thank you for nice suggestion, and I agree that could be useful. I will add a new formatting keyword for the total number of regions. so that you could make the output in that way using --format_snapshot_head.

provide a way to sort the regions (based on access rate or age)

I agree that this would be useful. Actually, this is already supported with --sort_regions_by option. As always saying, the command is not stable yet, but I will try to keep the feature.

sj-aws commented 11 months ago

Hi Honggyu,

I implemented the --format_snapshot_head keyword for the total number of regions in each snapshot: https://github.com/awslabs/damo/commit/92a81d0705085d58137ae6eeb7a4e601c6fa245a Please refer to the commit message for more information.

Please feel free to open this again if it doesn't solve your case, or there is something not answered.

honggyukim commented 11 months ago

Hi SeongJae,

Thanks very much for the work. But I worry that the interface doesn't allow me to use watch command. It fails when I run it with watch as follows.

# watch -n 1 ./damo show --format_snapshot_head 'nr: <number of regions>'
(... screen refreshed ...)
Every 1.0s: ./damo show --format_snapshot_head nr: <number of regions>

sh: -c: line 1: syntax error near unexpected token `newline'
sh: -c: line 1: `./damo show --format_snapshot_head nr: <number of regions>'

Can't we just print the "total number of regions" by default with this?

diff --git a/damo_show.py b/damo_show.py
index 01dbb61..9920e3e 100644
--- a/damo_show.py
+++ b/damo_show.py
@@ -227,6 +227,7 @@ def pr_records(args, records):
         snapshots = record.snapshots

         for sidx, snapshot in enumerate(snapshots):
+            print("total number of regions: %d" % len(snapshot.regions))
             format_pr(args.format_snapshot_head, args.min_chars_field, None,
                     None, snapshot, record, args.raw_number, mms)
             for r in snapshot.regions:

It shows output as follows.

# ./damo show
total number of regions: 11
0   addr [0 B         , 389.090 MiB) (389.090 MiB) access 0 %   age 8 m 12.200 s
1   addr [389.090 MiB , 588.641 MiB) (199.551 MiB) access 50 %  age 0 ns
2   addr [588.641 MiB , 2.265 GiB  ) (1.690 GiB  ) access 0 %   age 8 m 27.900 s
3   addr [2.265 GiB   , 3.962 GiB  ) (1.697 GiB  ) access 0 %   age 9 m 26.700 s
4   addr [3.962 GiB   , 5.651 GiB  ) (1.689 GiB  ) access 0 %   age 4 m 33.300 s
5   addr [5.651 GiB   , 7.343 GiB  ) (1.692 GiB  ) access 0 %   age 11 m 28.300 s
6   addr [7.343 GiB   , 9.028 GiB  ) (1.685 GiB  ) access 0 %   age 10 m 4.800 s
7   addr [9.028 GiB   , 10.722 GiB ) (1.694 GiB  ) access 0 %   age 12 m 52.400 s
8   addr [10.722 GiB  , 12.417 GiB ) (1.695 GiB  ) access 0 %   age 12 m 59.200 s
9   addr [12.417 GiB  , 14.102 GiB ) (1.686 GiB  ) access 0 %   age 13 m 0.400 s
10  addr [14.102 GiB  , 17.000 GiB ) (2.898 GiB  ) access 0 %   age 13 m 0.800 s
total size: 17.000 GiB

I feel there is no reason not to print this info by default. Could you please consider this? Thanks.

honggyukim commented 11 months ago

Please feel free to open this again if it doesn't solve your case, or there is something not answered.

FYI, it looks like I don't have a permission to reopen an issue. I don't see a button for that.

sj-aws commented 11 months ago

Thanks very much for the work. But I worry that the interface doesn't allow me to use watch command. It fails when I run it with watch as follows.

Seems that's an issue of watch or my command line rather than damo itself? I just confirmed modifying the command line like below works.

$ cat damo_show.sh
#!/bin/bash
./damo show --format_snapshot_head "nr: <number of regions>"
$ watch -n 1 sudo ./damo_show.sh

Some hacks like below would also be an option:

while :; do sudo ./damo show | head -n 40; sleep 1; done

Can't we just print the "total number of regions" by default with this?

I'd like to make the command flexible for more general case, and it's unclear if that information will be really important enough for everyone. So I'd say no at the moment.

it looks like I don't have a permission to reopen an issue.

Sorry, I didn't know. Please feel free to commenting on like this and ask me to open it again, or simply opening a new one.

honggyukim commented 11 months ago

Seems that's an issue of watch or my command line rather than damo itself? I just confirmed modifying the command line like below works.

Yes, I also wrote a short script for watch command.

I'd like to make the command flexible for more general case, and it's unclear if that information will be really important enough for everyone. So I'd say no at the moment.

OK. I asked because the option name looks a bit verbose to me so I felt it's difficult to remember. The option name could be a matter of preference so I understand that. Never mind then. Thanks.

I will see you soon in the chat series call.