Eliminate code duplication in memory statistics printing #841
Conversation
akosthekiss
added
enhancement
| stats.peak_pools_count, | ||
| stats.peak_allocated_chunks); | ||
| #endif /* MEM_STATS */ | ||
| mem_stats_print (); |
There was a problem hiding this comment.
@akiss77, for me it seems better to put mem_stats_print under #ifdef MEM_STATS, as without the define the call is no-op.
There was a problem hiding this comment.
@ruben-ayrapetyan I've been considering that as well. The reason why I proposed the above is that it remained semantically equivalent with the original version, which also left the call to mem_heap_print unguarded. Even in mem_heap_print, there are quite some code lines which are always compiled, and only a final portion is ifdef'd by MEM_STATS. If we choose to ifdef the call here, then maybe we should consider putting much more code under ifdef guards,
There was a problem hiding this comment.
@akiss77, I see. Thank you for your explanation.
The part of mem_heap_print that is not currently guarded by MEM_STATS, is not executed during the call, because the corresponding arguments are set to false, and only dump_stats argument is set to true.
So, maybe, its better to extract part of mem_heap_print that outputs statistics to another routine like mem_heap_print_stats, guard it with MEM_STATS, and call it from mem_heap_print and mem_stats_print. In the case, mem_stats_print could be guarded by #ifdef around the function, not inside.
What do you think about this?
akosthekiss
force-pushed
the
mem_stats_print
branch
from
1145288 to
c0f0b0a
Compare
|
@ruben-ayrapetyan I've updated the patch. I guess I got it according to your feedback. The only change I made is that I used the name |
JerryScript-DCO-1.0-Signed-off-by: Akos Kiss akiss@inf.u-szeged.hu
akosthekiss
force-pushed
the
mem_stats_print
branch
from
c0f0b0a to
7e36040
Compare
|
LGTM |
|
Landed (196e819) |
JerryScript-DCO-1.0-Signed-off-by: Akos Kiss akiss@inf.u-szeged.hu