Issue: #96 - Garbage collect inodeproc on each ui refresh.

[takeoverjp]

Hi @raboof ,

I've implemented garbage collection for inodeproc.
It releases the members for exited processes on each ui refresh.

My concern about this PR, is performance impact.
The time complexity of garbage_collect_inodeproc is O(MlogN), (M: inode number, N: process number).
In my desktop PC (Intel(R) Core(TM) i7-10510U CPU @ 1.80GHz), it takes around 1-4ms.
If you think it's critical, I guess we can thin out the garbage collection timing.

$ sudo nethogs -Cb | grep 'GC proc'
PERF: GC proctime: 1[ms]
PERF: GC proctime: 1[ms]
PERF: GC proctime: 1[ms]
PERF: GC proctime: 1[ms]
PERF: GC proctime: 2[ms]
PERF: GC proctime: 4[ms]

I'm not sure that this is the exact reason of #96 .
But when I've tried to reproduce memory leak with valgrind for 1 week,
almost all memory leak occur within members of inodeproc.

I know they are not leaked, because inodeproc hold their pointer.
But inode space is very huge, so I guess it's better to release unnecessary members.

$ sudo valgrind --leak-check=full nethogs
...
==5174== 21,580,283 (3,685,392 direct, 17,894,891 indirect) bytes in 76,779 blocks are definitely lost in loss record 86 of 88
==5174==    at 0x4C2E0EF: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5174==    by 0x40A1A1: setnode(unsigned long, int) (inode2prog.cpp:165)
==5174==    by 0x40A31C: get_info_by_linkname(char const*, char const*) (inode2prog.cpp:177)
==5174==    by 0x40A519: get_info_for_pid(char const*) (inode2prog.cpp:222)
==5174==    by 0x40A5B0: reread_mapping() (inode2prog.cpp:246)
==5174==    by 0x40A6E5: findPID(unsigned long) (inode2prog.cpp:263)
==5174==    by 0x406527: getProcess(unsigned long, char const*) (process.cpp:252)
==5174==    by 0x406D85: getProcess(Connection*, char const*, short) (process.cpp:378)
==5174==    by 0x403AFF: process_tcp(unsigned char*, pcap_pkthdr const*, unsigned char const*) (nethogs.cpp:153)
==5174==    by 0x4079FC: dp_parse_tcp (decpcap.c:128)
==5174==    by 0x407A61: dp_parse_ip (decpcap.c:164)
==5174==    by 0x407B3D: dp_parse_ethernet (decpcap.c:221)
==5174== 
==5174== 30,985,739 (5,219,040 direct, 25,766,699 indirect) bytes in 108,730 blocks are definitely lost in loss record 88 of 88
==5174==    at 0x4C2E0EF: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5174==    by 0x40A1A1: setnode(unsigned long, int) (inode2prog.cpp:165)
==5174==    by 0x40A31C: get_info_by_linkname(char const*, char const*) (inode2prog.cpp:177)
==5174==    by 0x40A519: get_info_for_pid(char const*) (inode2prog.cpp:222)
==5174==    by 0x40A5B0: reread_mapping() (inode2prog.cpp:246)
==5174==    by 0x406AE4: getProcess(Connection*, char const*, short) (process.cpp:330)
==5174==    by 0x403AFF: process_tcp(unsigned char*, pcap_pkthdr const*, unsigned char const*) (nethogs.cpp:153)
==5174==    by 0x4079FC: dp_parse_tcp (decpcap.c:128)
==5174==    by 0x407A61: dp_parse_ip (decpcap.c:164)
==5174==    by 0x407B3D: dp_parse_ethernet (decpcap.c:221)
==5174==    by 0x407C8B: dp_pcap_callback (decpcap.c:332)
==5174==    by 0x4E3FEF5: ??? (in /usr/lib/x86_64-linux-gnu/libpcap.so.1.7.4)
==5174== 
==5174== LEAK SUMMARY:
==5174==    definitely lost: 8,909,348 bytes in 185,512 blocks
==5174==    indirectly lost: 43,670,792 bytes in 185,227 blocks
==5174==      possibly lost: 5,540 bytes in 13 blocks
==5174==    still reachable: 450,517 bytes in 517 blocks
==5174==         suppressed: 0 bytes in 0 blocks

[raboof]

Cool, thanks a lot!

If you think it's critical, I guess we can thin out the garbage collection timing.

That might be good, since any delay may cause us to miss short-lived connections. Maybe do it once every 50 refreshes or so? (or if you want to go fancy perhaps even making it a commandline option?)

[takeoverjp]

Hi @raboof ,
Thank you for your review & comments.

Maybe do it once every 50 refreshes or so? (or if you want to go fancy perhaps even making it a commandline option?)

OK, I agree that every 50 refresh is enough for most cases and it might depend on use case.
So I'll add -g (garbage collection period) option.
The default value is 50.
And it can be disabled by -g 0.

If you have any comments about the new option, please let me know.

[takeoverjp]

HI @raboof
I've updated this PR as I've described above.
Please confirm again and please let me know if you have any comments.

[raboof]

great stuff, thanks!