On Thu, 17 Apr 2025 at 02:20, torikoshia <torikos...@oss.nttdata.com> wrote: > Regarding the implementation: > In the initial patch attached, I naïvely incremented the level just > before emitting the log line. > However, it might be cleaner to simply initialize the level variable to > 1 from the start. This could help avoid unnecessary confusion when > debugging that part of the code.
I didn't look at your patch before, but have now and agree that's not the best way. > Similarly, I noticed that in pg_get_process_memory_contexts(), the level > is initialized to 0 in ProcessGetMemoryContextInterrupt(void): > > int level = 0; > .. > MemoryContextStatsInternal(c, level, 100, 100, &grand_totals, .. > > If we want to be consistent, perhaps it would make sense to start from 1 > there as well. Yes. > BTW level variable has existed since before pg_backend_memory_contexts > was introduced — it was originally used for functions that help inspect > memory contexts via the debugger. Because of that, I think changing this > would affect not only these functions codes but some older ones. I get the impression that it wasn't well thought through prior to this. If you asked for max_level of 10 it would stop at 9. Changing these to 1-based levels means we'll now stop at level 10 without printing any more levels than we did before. The attached patch is how I think we should do it. David
make_memory_context_levels_1_based.patch
Description: Binary data
