search

damonitor / damo

DAMON user-space tool
GNU General Public License v2.0
18 stars 3 forks source link

damo_record_info: Check null case for heat_per_byte() #12

Closed m8 closed 2 months ago

m8 commented 2 months ago

simply fixes the null case for heats.

sjp38 commented 2 months ago

Hi @m8 Thank you for this PR! The change looks good to me. However, could you please clarify under what condition heats could be None, and add your Signed-off-by: (https://github.com/damonitor/damo/blob/next/CONTRIBUTING#L6) on the commit message?

m8 commented 2 months ago

Hi, this happens when region.nr_accesses.samples is 0 in line 134 and program continues and not setting the heats to 0 (line 140). I just changed the commit with setting heat to 0 while initializing.

sjp38 commented 2 months ago

Thank you for the clarification. I'd prefer keeping it None at declaration time, and doing the initialization on the constructor (__init__()), to keep the consistencies with similar other cases. What do you think? Also, could you please add the rational of the change on the commit message?

m8 commented 2 months ago

Ok then let me revert to be consistent with the codebase.

m8 commented 2 months ago

@sjp38 just fixed with regarding to your comments, thanks.

sjp38 commented 2 months ago

Thank you for the great work, @m8 ! Just merged.