Skip to content

Eliminate code duplication in memory statistics printing #841

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Eliminate code duplication in memory statistics printing #841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 7 additions & 22 deletions jerry-core/mem/mem-allocator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,29 +51,14 @@ mem_finalize (bool is_show_mem_stats) /**< show heap memory stats
{
mem_pools_finalize ();

#ifdef MEM_STATS
if (is_show_mem_stats)
{
mem_heap_print (false, false, true);

#ifdef MEM_STATS
mem_pools_stats_t stats;
mem_pools_get_stats (&stats);

printf ("Pools stats:\n");
printf (" Chunk size: %zu\n"
" Pools: %zu\n"
" Allocated chunks: %zu\n"
" Free chunks: %zu\n"
" Peak pools: %zu\n"
" Peak allocated chunks: %zu\n\n",
MEM_POOL_CHUNK_SIZE,
stats.pools_count,
stats.allocated_chunks,
stats.free_chunks,
stats.peak_pools_count,
stats.peak_allocated_chunks);
#endif /* MEM_STATS */
mem_stats_print ();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akiss77, for me it seems better to put mem_stats_print under #ifdef MEM_STATS, as without the define the call is no-op.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

}
#else /* MEM_STATS */
(void) is_show_mem_stats;
#endif /* !MEM_STATS */

mem_heap_finalize ();
} /* mem_finalize */
Expand Down Expand Up @@ -158,13 +143,13 @@ mem_stats_reset_peak (void)
void
mem_stats_print (void)
{
mem_heap_print (false, false, true);
mem_heap_stats_print ();

mem_pools_stats_t stats;
mem_pools_get_stats (&stats);

printf ("Pools stats:\n");
printf (" Chunk size: %zu\n"
printf (" Chunk size: %zu\n"
" Pools: %zu\n"
" Allocated chunks: %zu\n"
" Free chunks: %zu\n"
Expand Down
43 changes: 26 additions & 17 deletions jerry-core/mem/mem-heap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -944,23 +944,7 @@ mem_heap_print (bool dump_block_headers, /**< print block headers */
#ifdef MEM_STATS
if (dump_stats)
{
printf ("Heap stats:\n");
printf (" Heap size = %zu bytes\n"
" Chunk size = %zu bytes\n"
" Allocated chunks count = %zu\n"
" Allocated = %zu bytes\n"
" Waste = %zu bytes\n"
" Peak allocated chunks count = %zu\n"
" Peak allocated = %zu bytes\n"
" Peak waste = %zu bytes\n",
mem_heap_stats.size,
MEM_HEAP_CHUNK_SIZE,
mem_heap_stats.allocated_chunks,
mem_heap_stats.allocated_bytes,
mem_heap_stats.waste_bytes,
mem_heap_stats.peak_allocated_chunks,
mem_heap_stats.peak_allocated_bytes,
mem_heap_stats.peak_waste_bytes);
mem_heap_stats_print ();
}
#else /* MEM_STATS */
(void) dump_stats;
Expand Down Expand Up @@ -1108,6 +1092,31 @@ mem_heap_stat_free (size_t first_chunk_index, /**< first chunk of the freed area
mem_heap_stats.allocated_bytes -= bytes;
mem_heap_stats.waste_bytes -= waste_bytes;
} /* mem_heap_stat_free */

/**
* Print heap statistics
*/
void
mem_heap_stats_print (void)
{
printf ("Heap stats:\n");
printf (" Heap size = %zu bytes\n"
" Chunk size = %zu bytes\n"
" Allocated chunks count = %zu\n"
" Allocated = %zu bytes\n"
" Waste = %zu bytes\n"
" Peak allocated chunks count = %zu\n"
" Peak allocated = %zu bytes\n"
" Peak waste = %zu bytes\n",
mem_heap_stats.size,
MEM_HEAP_CHUNK_SIZE,
mem_heap_stats.allocated_chunks,
mem_heap_stats.allocated_bytes,
mem_heap_stats.waste_bytes,
mem_heap_stats.peak_allocated_chunks,
mem_heap_stats.peak_allocated_bytes,
mem_heap_stats.peak_waste_bytes);
} /* mem_heap_stats_print */
#endif /* MEM_STATS */

/**
Expand Down
1 change: 1 addition & 0 deletions jerry-core/mem/mem-heap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ typedef struct

extern void mem_heap_get_stats (mem_heap_stats_t *);
extern void mem_heap_stats_reset_peak (void);
extern void mem_heap_stats_print (void);
#endif /* MEM_STATS */

#ifdef JERRY_VALGRIND_FREYA
Expand Down