awslabs / damo

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

show: status: Print addr info in hex format #67

Open honggyukim opened 11 months ago

honggyukim commented 11 months ago

The current output of damo show and status do not print address info in hex format. It makes the address comparison with other format difficult.

This patch makes printing address info in hex format.

Before:

  # ./damo show
  0   addr [4.000 GiB   , 5.299 GiB  ) (1.299 GiB  ) access 0 %   age 29 s
  1   addr [5.299 GiB   , 6.594 GiB  ) (1.295 GiB  ) access 0 %   age 2 m 12.300 s
  2   addr [6.594 GiB   , 7.892 GiB  ) (1.299 GiB  ) access 0 %   age 2 m 12.300 s
      ...

After:

  $ ./damo show
  0   addr [100000000   , 1531b7000  ) (1.299 GiB  ) access 0 %   age 29.100 s
  1   addr [1531b7000   , 1a5ff6000  ) (1.295 GiB  ) access 0 %   age 36.900 s
  2   addr [1a5ff6000   , 1f91e4000  ) (1.299 GiB  ) access 0 %   age 36.900 s
      ...
sj-aws commented 11 months ago

Thank you for this nice PR! I agree the hex format could be more useful for many cases. Nevertheless, I think decimal format could still be useful for other cases.

Are you having the problem due to the decimal address? Or, the human friendly abstracted output that doesn't provide exact byte-level address? If the case is the latter and you only need byte-level address, you could use --raw option and convert the output to hex format. Or, conver the output of other tool that using hex format to decimal.

If use of decimal address is really a problem, I think we could add an option for printing it in hex foramt. E.g., --hex_addr?

honggyukim commented 11 months ago

Hi SeongJae, thanks for the review.

I think decimal format could still be useful for other cases.

But I haven't seen some cases that represent address info in decimal format in other projects.

Are you having the problem due to the decimal address?

We feel it looks awkward to see address in decimal format so always have to translate in hex again to compare with addresses that are from /proc/iomem and dmesg output. Moreover, address info is normally aligned to hex unit in most cases.

Or, the human friendly abstracted output that doesn't provide exact byte-level address?

I haven't see a case representing address as GiB, MiB, or etc because those units are used for representing size info.

If use of decimal address is really a problem, I think we could add an option for printing it in hex foramt. E.g., --hex_addr?

I'm wondering if we can make hex address as default and provide an option --decimal_addr as an option instead.

I don't fully know the background of your decision making the address in decimal format, but it'd be helpful if you could explain if there is a strong reason for that. Thanks.

honggyukim commented 11 months ago

I've been thinking again. Printing both formats might be another solution as follows.

  $ ./damo show
  0   addr [100000000 (4.0GiB)   , 1531b7000 (5.3GiB)  ) (1.299 GiB  ) access 0 %   age 29.100 s
      ...
sj-aws commented 11 months ago

Hi Honggyu,

I use the decimal size unit for representing address because I believe it is easier for humans who have ten fingers in usual, or, at least for me. Compared to hexadecimal addresses, decimal size unit is easier to memorize, pronounce, and compare. It especially makes it easier when we discuss about the damo output with others offline. It helps people to easily understand how big/small the memory region is compared to other, how far it is separated from another region, where in the entire memory the region is located, etc. Some people might smart enough to deal with hexadecimal numbers, but at least I'm not.

IMHO, others using hex shouldn't be a reason for us to do same.

I'm also negative about making hexadecimal by default. I think finding the option wouldn't be a big challenge for the people who smart enough to deal with hexadecimal.

Printing both formats could also be a good option, But doing so by default might be too verbose.

We can think about supporting hex or hex+decimal address in easier way. damo show will provide a flexible output formatting feature, which would be similar to --pretty of git log. Current version of damo already have early version of the feature. Note that the interface can suddenly change, until be documented. We can add another identifier for hexadecimal addresses, e.g., <start address hex> and <end address hex>. Then, damo show --format_region "[<start address hex> (<start address>), <end address hex> (<end address>)) would make output as you suggested.

We could also further let user to define the format in file, and let damo show to use a format file of specific name to be used by default, if available.

Again, please note that damo show is not yet documented and therefore subject to be changed quite a lot. ETA of the documentation is hopefully OSSummit EU 2023.

honggyukim commented 11 months ago

Thanks for your explanation. Then I will think about adding --hex_addr option instead.